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

Updated Bleed for iOS #4780

Merged
merged 6 commits into from
Sep 24, 2020
Merged

Updated Bleed for iOS #4780

merged 6 commits into from
Sep 24, 2020

Conversation

jwoo-msft
Copy link
Member

@jwoo-msft jwoo-msft commented Sep 16, 2020

Related Issue

Fixed #4745

Description

Setting the constraint with superview for bleed resulted in dependency on the superview. The fix limited the use of constraint to superview. Made explicit reference to the superview for code maintenance.

How Verified

  1. Updated Bleed.Comprehensive.json to include a container with bleed in a showcard.
  2. Ran regression tests, and verified the fix
Microsoft Reviewers: Open in CodeFlow

@shalinijoshi19
Copy link
Member

@jwoo-msft this might need to be ported to release/1.2 - let's confirm with the Teams mobile team tonight. Thanks!

Copy link
Member

@shalinijoshi19 shalinijoshi19 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just an iOS issue? @RebeccaAnne should have a peek here too !

@RebeccaAnne
Copy link
Contributor

Is this just an iOS issue? @RebeccaAnne should have a peek here too !

I'm not fully grokking the nuance of these changes, but the crash from the bug looks like an iOS specific concern.

That said, I ran the bug payload through UWP, .NET, and JavaScript, and it doesn't look like it's rendering quite the same on all platforms. UWP seems to bleed the show card into the show card padding, whereas the other two don't. @jwoo-msft what is this card supposed to look like?

UWP (note no emphasis padding around the show card):
image

JavaScript (emphasis border around the show card) (.NET is similar):
image

@jwoo-msft jwoo-msft merged commit 91267aa into main Sep 24, 2020
@jwoo-msft jwoo-msft deleted the jwoo/ios-bleed-update branch September 24, 2020 06:59
@jwoo-msft
Copy link
Member Author

Is this just an iOS issue? @RebeccaAnne should have a peek here too !

I'm not fully grokking the nuance of these changes, but the crash from the bug looks like an iOS specific concern.

That said, I ran the bug payload through UWP, .NET, and JavaScript, and it doesn't look like it's rendering quite the same on all platforms. UWP seems to bleed the show card into the show card padding, whereas the other two don't. @jwoo-msft what is this card supposed to look like?

The card above can be condensed to the below. FYI, I added the snippet of this card to our Bleed.Comprehenisve.json.

		{
			"type":"TextBlock",
			"text":"REMOVED TEXT",
			"size":"ExtraLarge",
			"weight":"Bolder",
			"wrap":true
		},
		{
			"type":"ActionSet",
			"actions":[
			{
				"type":"Action.ShowCard",
				"title":"Results",
				"card":{
					"type":"AdaptiveCard",
					"$schema":"http://adaptivecards.io/schemas/adaptive-card.json",
					"body":[
					{
						"type":"Container",
						"items":[
							{
								"type":"Container",
								"items":[
								{
									"type":"TextBlock",
									"text":"REMOVED TEXT",
									"fontType":"Default",
									"size":"ExtraLarge",
									"spacing":"None",
									"weight":"Bolder",
									"wrap":true
								}
								],
								"style":"default"
							}
						],
						"style":"emphasis",
						"bleed":true
					}
					]
				}
			}
		]
		}

ActionSet ShowCard -> Container with emphasis style with bleed true -> Container with default style.
I don't think we have specified the bleed for showcard in ActionSet. If the parent card has a default style, then letting the container with emphasis to bleed is a possibility.

jwoo-msft added a commit that referenced this pull request Oct 22, 2020
* Fixed the crash issue

* Fixed Bleeding Crash Issue

* finishing up changes for bleed update

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
@jwoo-msft jwoo-msft mentioned this pull request Oct 22, 2020
shalinijoshi19 added a commit that referenced this pull request Oct 23, 2020
* Fixed the crash issue

* Fixed Bleeding Crash Issue

* finishing up changes for bleed update

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
* Fixed the crash issue

* Fixed Bleeding Crash Issue

* finishing up changes for bleed update

Co-authored-by: shalinijoshi19 <shalinij@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS][Rendering] Crash when "bleed" property used on outermost container
3 participants