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

Using word-wrap in Adaptive Cards ActionSet and markdown list #4075

Closed
avrum opened this issue Oct 20, 2021 · 10 comments · Fixed by #4084
Closed

Using word-wrap in Adaptive Cards ActionSet and markdown list #4075

avrum opened this issue Oct 20, 2021 · 10 comments · Fixed by #4084
Assignees
Labels
backlog Out of scope for the current iteration but it will be evaluated in a future release. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. ExemptFromDailyDRIReport exempt from daily DRI report external-adaptive-cards

Comments

@avrum
Copy link

avrum commented Oct 20, 2021

I've been having issue with word-wrap property when using ActionSet with mark down list.

For some reason when I'm including a markdown list in the "ShowCard" the text get the wrap property but the boundaries seems to be shifted to the right.

screenshot that demonstrate this issue:

enter image description here

When viewing in in dev mode (marking the component edge):

enter image description here

This works as expected without the bullets(list), but I need this design to work with bullets

The original card JSON:

{
	"contentType": "application/vnd.microsoft.card.adaptive",
	"content": {
		"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
		"type": "AdaptiveCard",
		"version": "1.2",
		"body": [
			{
				"type": "TextBlock",
				"size": "Large",
				"weight": "Bolder",
				"text": "Emergency Ambulance"
			},
			{
				"type": "TextBlock",
				"size": "Medium",
				"weight": "Bolder",
				"text": "Care Instructions:"
			},
			{
				"type": "ActionSet",
				"actions": [
					{
						"type": "Action.ShowCard",
						"title": "Call an Ambulance",
						"card": {
							"type": "AdaptiveCard",
							"body": [
								{
									"type": "TextBlock",
									"text": "- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n",
									"wrap": true
								}
							]
						}
					}
				]
			}
		]
	}
}

This seems to work as expected in "https://adaptivecards.io/designer/" :

enter image description here

but in BotFramework-WebChat recent version it doesn't work as expected, and we have lots of customers suffering from this bug

@avrum avrum added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-reported Required for internal Azure reporting. Do not delete. labels Oct 20, 2021
@carlosscastro
Copy link
Member

@compulim could you please triage?

@compulim
Copy link
Contributor

compulim commented Oct 21, 2021

Thanks @avrum for the repro, this is very useful.

Will investigate.

@avrum
Copy link
Author

avrum commented Oct 21, 2021

@compulim you're welcome :)
Thank you guys for the fast response.

@compulim
Copy link
Contributor

compulim commented Oct 21, 2021

Some early investigations. I have a tool called "Adaptive Card loader", which load AC JSON outside of Web Chat. But also have an option to pass styling (a.k.a. AC Host Config). I use it to verify if it is an issue on AC vs. Web Chat.

Here is my quick tool, https://compulim.github.io/adaptive-card-loader/. Check the "Enable Markdown" checkbox.

image

With default styling, the <ul> also leaked.

But if I manually removed width: 100%, it works.

image

Need more investigations to see if it's AC vs. Web Chat.

@compulim
Copy link
Contributor

compulim commented Oct 21, 2021

Status: Investigated

Background

Adaptive Card Designer has this CSS rule and it assume the hosting app must run in border-box mode:

#designerRootHost * {
  box-sizing: border-box;
}

However, in Web Chat, we never assume our host is in border-box mode (CSS standard default is content-box). Thus, when Web Chat use AC library to render the <ul>, AC added width: 100%, and the content bleed due to miscalculation.

Solutions

I need some time thinking if we should do one of the followings:

Web Chat: add the following CSS

To workaround, or to align with AC's expectation of its host always in border-box mode:

.ac-container * {
  box-sizing: border-box;
}

Or

.ac-textBlock * {
  box-sizing: border-box;
}

Or

.ac-textBlock ol, .ac-textBlock ul {
  box-sizing: border-box;
}

Sidenote: AC is extensible and support custom controls. If we use .ac-container * selector, it may mess up with custom controls.

Adaptive Cards: don't assume host is always in border-box mode

We could ask AC team not to assume the host is always in border-box mode, because CSS standard default is content-box.

If they assume it's always in border-box mode, they could add it to every of their rendered element.

Adaptive Cards: add box-sizing: border-box to <ul>

If AC team add box-sizing: border-box to <ul>, it will fix the issue.

They could add it around AdaptiveCards/card-elements.ts:1069.

image

@carlosscastro carlosscastro added the customer-replied-to Required for internal reporting. Do not delete. label Oct 22, 2021
@lauren-mills lauren-mills added the backlog Out of scope for the current iteration but it will be evaluated in a future release. label Nov 3, 2021
@lauren-mills
Copy link

@compulim - adding this to the backlog since it seems like you have a couple of options for resolutions.

@compulim
Copy link
Contributor

compulim commented Nov 3, 2021

I just reached David from AC team and they suggest adding box-sizing: border-box to the whole AC component.

I will add the following CSS stylesheet:

.webchat__adaptive-card-renderer {
  box-sizing: border-box;
}

One more thing, this bug only applies to Adaptive Cards when the first element of the Markdown is a list.

It repro:

{
  "type": "TextBlock",
  "text": "- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n",
  "wrap": true
}

And it don't repro:

{
  "type": "TextBlock",
  "text": "Hello, World!\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n- The is a very long text, one two three four five six seven eight nine ten, eleven twelve thirteen fourteen .\n\n",
  "wrap": true
}

@compulim
Copy link
Contributor

compulim commented Nov 4, 2021

While fixing the issue, I found that AC is applying some styles only for the first child element. These styles are not added by Markdown-It but Adaptive Cards.

Since Web Chat don't have a test that only a single list in Adaptive Card, thus, we never catch this bug. I am adding a test to prevent regression.

@avrum a temporary workaround here would be adding a line of text before the list.

Technical details

If the Markdown list is the first thing in the TextBlock, then AC apply some style to the <ul> and it breaks word wrap due to its assumption of border-box mode.

  • Styles applied to the first item
    • style="margin-top: 0px; width: 100%; overflow: hidden; text-overflow: ellipsis;"
  • Styles applied to the last item
    • style="margin-bottom: 0px;"

If paragraph is the first thing in the Markdown

image

If unordered list is the first thing in the Markdown

image

@avrum
Copy link
Author

avrum commented Nov 8, 2021

Thank you guys for fixing this issue,
Let me know when it is merged and I can consume the new release with the fix.

@mrivera-ms mrivera-ms added the ExemptFromDailyDRIReport exempt from daily DRI report label Nov 22, 2021
@avrum
Copy link
Author

avrum commented Dec 20, 2021

Thanks again for fixing this 🪲 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Out of scope for the current iteration but it will be evaluated in a future release. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. ExemptFromDailyDRIReport exempt from daily DRI report external-adaptive-cards
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants