Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adaptive Card 1.2 style "positive" and "destructive" is not working #2145

Closed
compulim opened this issue Jul 4, 2019 · 9 comments · Fixed by #2810
Closed

Adaptive Card 1.2 style "positive" and "destructive" is not working #2145

compulim opened this issue Jul 4, 2019 · 9 comments · Fixed by #2810
Assignees
Labels
bug Indicates an unexpected problem or an unintended behavior. front-burner needs-design-input UX/UI design item p1 Painful if we don't fix, won't block releasing

Comments

@compulim
Copy link
Contributor

compulim commented Jul 4, 2019

Description

Styling an action in Adaptive Card using "positive" and "destructive".

You can copy the JSON below, save it to a file, then upload it to MockBot. MockBot will render it as an Adaptive Card.

{
    "type": "AdaptiveCard",
    "body": [
        {
            "type": "TextBlock",
            "text": "Hello, World!"
        }
    ],
    "actions": [
        {
            "type": "Action.Submit",
            "title": "Submit",
            "style": "destructive"
        }
    ],
    "$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
    "version": "1.0"
}

Screenshots

Screenshot from Adaptive Card Designer

image

Screenshot from Web Chat Playground

image

Screenshot of the DOM tree

image

As you can see here, there is a class named style-destructive. I believe this will turn the button red. But looks like we did not load the CSS required for this style.

Version

I am using "master" branch of Web Chat. Verified it is using AC 1.2 package.

To Reproduce

Steps to reproduce the behavior:

  1. Save the JSON above and name it destructive.json
  2. Navigate to https://webchat-playground.azurewebsites.net/
  3. Uplaod the JSON file to MockBot

Expected behavior

MockBot should reply with an Adaptive Card with the button marked as red. Similar to the screenshot from Adaptive Card Designer.

Additional context

It could be Adaptive Card 1.2 has a new CSS and we did not load the CSS. This is because there is a class "style-destructive" but it was not styled by any CSS rules.

Feature list on Adaptive Card 1.2 include "action sentiment", see it at microsoft/AdaptiveCards#2444.

[Bug]

@compulim compulim added bug Indicates an unexpected problem or an unintended behavior. Pending labels Jul 4, 2019
@tdurnford
Copy link
Contributor

The Direct Line Connector Service uses Adaptive Cards v1.0.6, so it is not able to handle features introduced in Adaptive Cards v1.1.0 or greater. As a result, it drops items/media in the card like positive/destructive and videos that were implemented in the newer versions.

Even though the Direct Line Connector Service does not support v1.2 of Adaptive Cards, there is a pretty suave work around. microsoft/BotFramework-Services#87 (comment)

@tdurnford tdurnford self-assigned this Jul 8, 2019
@tdurnford
Copy link
Contributor

tdurnford commented Jul 8, 2019

Unfortunately, the work around didn't work in this case. The style type for the button when it reaches Web Chat is still "destructive." I will investigate this further. @compulim is probably correct that the style sheet isn't being loaded properly.

@tdurnford
Copy link
Contributor

tdurnford commented Jul 9, 2019

Per Adaptive Cards PR microsoft/AdaptiveCards#861, the positive/destructive styles "should be done using native styling." Looking at the Adaptive Cards Designer's CSS for Web Chat, they add CSS rules for the style-destructive and style-positiveclasses.

.ac-pushButton.style-positive {
    background-color: #0078D7;
    color: white;
}

.ac-pushButton.style-positive:hover, .ac-pushButton.style-positive:active {
    background-color: #006ABC;
}

.ac-pushButton.style-destructive {
    background-color: #E50000;
    color: white;
}

.ac-pushButton.style-destructive:hover, .ac-pushButton.style-destructive:active {
    background-color: #BF0000;
}

Adding these CSS rules to the client HTML styling leads to the expected result, but we should incorporate them in to the style set. Should we also consider adding the corresponding colors for positive and destructive in to the default style options or just set them to a fixed color?

image

@compulim
Copy link
Contributor Author

compulim commented Jul 9, 2019

Thanks for investigating @tdurnford.

Interesting to see if we should take/absorb their CSS in Web Chat or not. I would run thru each CSS styles and weight according to these criteria:

  • Why they need to be specified in this "Web Chat CSS" file?
  • Are they conflicting with our own styles?
  • Are they following our design language? E.g. border thickness/roundness, subtle-ness of the border, etc.

On the other hand, @DesignPolice could you comment on the color? tl;dr Adaptive Cards introduced a way to set the background color of the button. One is fill it with accent color (a.k.a. positive), another one is to fill it in red (a.k.a. destructive). Do you think their red color fit our accent color?

@corinagum
Copy link
Contributor

@tdurnford already had a convo with @DesignPolice afaik

@DesignPolice
Copy link

@compulim we selected a hover color for the blue, (you are correct the red is very strong), but I will address all of the colors and buttons in the Fluent update to WebChat. I'm waiting on some accessibility changes from the Fabric team, it failed on many levels. They should update in a few weeks. So one set of changes instead of lots of little ones. I will keep you posted on that status.

@corinagum
Copy link
Contributor

Let's just choose accent blue (Emulator blue) and the dark accessible red Web Chat is already using for other error messages and get this one out. When design gets to it, it will be really quick to just update the colors.

@corinagum corinagum added the R8 label Dec 12, 2019
@tdurnford tdurnford reopened this Dec 26, 2019
@tdurnford
Copy link
Contributor

Unintentionally closed issue

@cwhitten cwhitten added the p1 Painful if we don't fix, won't block releasing label Jan 9, 2020
@compulim compulim mentioned this issue Mar 5, 2020
40 tasks
@yogendrajs
Copy link

I could see this not working in Adaptive cards in Teams bot-framework, any idea?
Found this - https://stackoverflow.com/questions/65290714/how-to-style-action-in-ms-teams-adaptive-card but no success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or an unintended behavior. front-burner needs-design-input UX/UI design item p1 Painful if we don't fix, won't block releasing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants