BotFramework-WebChat icon indicating copy to clipboard operation
BotFramework-WebChat copied to clipboard

Card backgrounds should be different than text backgrounds

Open xtianus79 opened this issue 6 years ago • 16 comments

Feature Request

I am noticing that when I change the text background the card background changes the same. These should 2 entities should be separate in their core styling. This is very important. If I change background color, border and radius that should not also affect my adaptive card background.

would you be able to seperate the concerns?

[Enhancement]

xtianus79 avatar Jul 27 '19 07:07 xtianus79

@xtianus79 Can you add the specific styling options you are passing to Web Chat and screenshots of how the is Adaptive Card rendering with those changes. I believe a lot of the Web Chat styling options carryover to Adaptive Cards to keep the styling consistent throughout the component.

tdurnford avatar Jul 27 '19 09:07 tdurnford

@tdurnford the styling option carrying over to a certain extent makes sense but in my thoughts it shouldn't be the exact same as the text but separate. The screen below illustrates the point. I wouldn't want a grey background here and I surely want it to have a border radius... I would want it to just be the same color as the background.

bot-styling-example2

xtianus79 avatar Jul 27 '19 16:07 xtianus79

Looking at the text and attachment as components, both are wrapped by Bubble, which is what has the background color that is assignable via defaultStyleOptions. For simplicity, I believe that not adding another potentially confusing style option is preferable, as using idiosyncratic styling or middleware is a reasonable workaround for differentiating these backgrounds if the user think changing the background colors is necessary.

My inclination is to close this request but consider reopening if this garners significant attention from our users. @compulim, thoughts?

corinagum avatar Jul 27 '19 20:07 corinagum

@xtianus79 good suggestion, and I don't think you would be the only user who would benefit from allowing style overrides like the ones you are asking for. this would likely sit on our backlog for the next few months, but if you opened a pull request with change we'd prioritize reviewing and shepherding the change into an official release.

cwhitten avatar Jul 28 '19 02:07 cwhitten

@cwhitten sure. When I open the pull request will it he a feature request?

xtianus79 avatar Jul 31 '19 13:07 xtianus79

Few things we should look at when implementing the fix. We should use Adaptive Card host config as much as we could.

  1. Can we set background color using host config?
    • If yes, in Web Chat default host config, we should send the background color, and let developer override it using Adaptive Card host config (not style options)
    • If no, we need a style options here
      • Should the style options limited to Adaptive Card, or all attachments? (TBD)
  2. Can the developer set background color of a button in Adaptive Cards?
    • If yes, I think the better solution for @xtianus79 is to change the background color of a button, this is for matching the dark theme in their app
    • If no, the developer should set the background color for the card (which is point 1) and it makes their UI looks more consistent
      • We should also file a bug to Adaptive Card, saying the button color should be customizable, or they should have a dark theme

compulim avatar Aug 12 '19 20:08 compulim

It's not documented in the Adaptive Cards Host Config Schema, but you can set the background color of Adaptive Cards through the host config. I believe you have to pass the whole host config to Web Chat though. It's not configured to do a deep merge at the moment.

Web Chat

const adaptiveCardHostConfig  = {
  ...
  "containerStyles": {
      "backgroundColor": "#0063B1"
      ....
    }
    ....
};

renderWebChat({
  adaptiveCardHostConfig,
  directLine
}, document.getElementById('webchat'));

Screenshot image

tdurnford avatar Aug 12 '19 20:08 tdurnford

TJ is correct in that the entire HostConfig must be passed in. While adaptive cards do not require any of the properties, if any are removed it will break in web chat.

I do have the card styling working via updating the HostConfig and passing it in. However, unless I misread the initial request, he also wants to style the border differently when an adaptive card is used.

In this scenario, I have gone thru and made changes to web chat (an unsubmitted PR, at this point) that allows the developer to override the following items. In this way, the user can disable the bubble border around an adaptive card and enable the cards border which appears the same (I haven't run tests on this, yet). These are all properties that we allow developers to override elsewhere (bubble, suggested actions, etc.).

This does reach beyond the customer's ask by introducing styling options for buttons and images, but I couldn't see introducing options for only select areas of cards. Certain properties, like button text size or weight, are left to be configured via the adaptive card or other methods.

adaptiveCardBackgroundColor, adaptiveCardBorderColor, adaptiveCardBorderRadius, adaptiveCardBorderStyle, adaptiveCardBorderWidth, adaptiveCardButtonBackgroundColor, adaptiveCardButtonBorder, adaptiveCardButtonTextColor, adaptiveCardDisabledButtonBackgroundColor, adaptiveCardDisabledButtonBorder, adaptiveCardDisabledButtonTextColor, adaptiveCardImageMaxHeight, adaptiveCardImageMinWidth, adaptiveCardImageWidth, adaptiveCardTextColor

image

stevkan avatar Aug 12 '19 21:08 stevkan

I think the bubble border is owned by Web Chat and should not be removed for Adaptive Cards. This is because Adaptive Cards own the content, not the container.

And for additional styles, developers should use Adaptive Card host config. This includes button colors, image heights, etc. If these styles cannot be tweaked using Adaptive Card host config, a new issue should be filed to Adaptive Card to have it fixed. The reason is, Web Chat do not own the renderer of Adaptive Card. It is the renderer responsibility to render the button and provide the content to Web Chat.

compulim avatar Aug 12 '19 22:08 compulim

Coupling the Adaptive Cards styling to our style options is not recommended in my opinion. That would mean that we would much more closely need to monitor any and all changes the AC repo makes on their styling.

Instead of making the entire Adaptive Card style-able in the defaultStyleOptions, I think the direction we want to go in is to allow the changing of the background color and border color/style/width of the BUBBLE component based on whether the bubble is activity or attachment. (Note, this is NOT differentiating activity type).

This means activities of type text could be differentiated from activity attachments. This does not mean that Adaptive Cards themselves will be style-able in defaultStyleOptions.

The changes to defaultStyleOptions would be the following:

  bubbleFromBotTextBackground: '...',
  bubbleFromBotTextBorderColor: '...',
  bubbleFromBotTextBorderRadius: '...',
  bubbleFromBotTextBorderStyle: '...',
  bubbleFromBotTextBorderWidth: '...',
  bubbleFromBotAttachmentBackground: '...',
  bubbleFromBotAttachmentColor: '...',
  bubbleFromBotAttachmentBorderRadius: '...',
  bubbleFromBotAttachmentBorderStyle: '...',
  bubbleFromBotAttachmentBorderWidth: '...',
  bubbleFromUserTextBackground: '...',
  bubbleFromUserTextBorderColor: '...',
  bubbleFromUserTextBorderRadius: '...',
  bubbleFromUserTextBorderStyle: '...',
  bubbleFromUserTextBorderWidth: '...',
  bubbleFromUserAttachmentBackground: '...',
  bubbleFromUserAttachmentColor: '...',
  bubbleFromUserAttachmentBorderRadius: '...',
  bubbleFromUserAttachmentBorderStyle: '...',
  bubbleFromUserAttachmentBorderWidth: '...',

This would mean that the old styleOptions (bubbleBackground) would need to be deprecated.

corinagum avatar Aug 12 '19 22:08 corinagum

@tdurnford tagging you as related to convo you had with @compulim

corinagum avatar Sep 08 '19 00:09 corinagum

My understanding is that this should be complete in the next couple of days. Please communicate in the event it is not.

CoHealer avatar Sep 19 '19 21:09 CoHealer

I'm returning this to Backlog due to inactivity. Feel free to resume discussion here. :)

corinagum avatar Oct 04 '19 18:10 corinagum

Loading up for discussion and prioritization of the upcoming milestone

cwhitten avatar Feb 26 '21 00:02 cwhitten

Didn't make it onto the R13 list, but I've added the front-burner label for consideration for upcoming milestones.

corinagum avatar Mar 10 '21 23:03 corinagum

Can anyone tell me how I can change the font and color of this adaptiveCard. image

PLepping avatar May 16 '22 12:05 PLepping