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

[Android] Verify correct behavior of bleed property #2700

Merged
merged 3 commits into from
Apr 18, 2019

Conversation

almedina-ms
Copy link
Contributor

Updated bleed behavior for android and consolidated some logic into functions so the padding and bleed logic would not be repeated

How verified

The mobile app was run and these are the results

Comprehensive payload

Screenshot_19700120-212829
Screenshot_19700120-212834
Screenshot_19700120-212841

Column.Bleed

Screenshot_19700120-212846

ColumnSet.Bleed

Screenshot_19700120-212852

Container.Bleed

Screenshot_19700120-212857

marginLeft = -padding;
}

if(bleedDirection == ContainerBleedDirection.BleedToTrailing || bleedDirection == ContainerBleedDirection.BleedToBothEdges)
Copy link
Member

@paulcam206 paulcam206 Apr 17, 2019

Choose a reason for hiding this comment

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

nit: space after if #Resolved

@dclaux
Copy link
Member

dclaux commented Apr 17, 2019

Per the screenshots, there seems to still be a couple things not handled as expected. Namely, bleeding at the top/bottom of containers/the card itself doesn't seem to be handled. A container that bleeds should bleed not only left and right, but also top and bottom when appropriate. In the first screenshot for instance, the "Bleeding container at the top of the card" should bleed into the top padding of the card.

I am going to assume that this has been compared to the Designer's rendering; the designer voluntarily disables bleeding top and bottom in order to leave space at the top and bottom of any container onto which elements can be dragged (otherwise, it would be impossible to reorder elements.) To see final render of the card, use the "Preview mode" button. Or, use the visualizer instead.

@RebeccaAnne
Copy link
Contributor

We don't currently have that behavior on any of our non-javascript renderers, as we were all using the designer as a comparison point, and it's not super explicit in the spec. It does look like the last gif example shows that behavior (I assume "marginFromParent":"none" is an old way to say bleed?), but looks like we missed it. I'll log a bug to track addressing it separate of this PR.


In reply to: 484155715 [](ancestors = 484155715)

@RebeccaAnne
Copy link
Contributor

#2702

Also, i understand the concern about needing the visualizer to be usable, but are we concerned that users won't realize that what they see in the visualizer is not what will ultimately be rendered? (Also, where is the "Preview mode" button? I can't seem to find it.)


In reply to: 484190751 [](ancestors = 484190751,484155715)

@dclaux
Copy link
Member

dclaux commented Apr 17, 2019

I believe the test payload you guys used is from here: #2109

There is also a screenshot of the rendered result in there; that is the reference.

@dclaux
Copy link
Member

dclaux commented Apr 17, 2019

Yes, there is a risk of potential confusion, and I have ideas for making it better. However these improvements will come later, and in general the designer should NOT be used as a reference for the final visuals (unless using the "Preview mode" feature in the 2.0 early preview) because the designer will always behave slightly differently in some cases to facilitate drag and drop.

@almedina-ms almedina-ms merged commit ed3774e into master Apr 18, 2019

public static void ApplyPadding(ContainerStyle elementContainerStyle, ContainerStyle parentContainerStyle, LinearLayout collectionElementView, Context context, HostConfig hostConfig)
{
if (elementContainerStyle != parentContainerStyle)
Copy link
Member

Choose a reason for hiding this comment

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

just want to let you know that you can call GetPadding() on collectionType object to see if it gets padding.

@almedina-ms almedina-ms deleted the user/almedina-ms/AndroidVerifyBleedProperty branch May 2, 2019 22:03
rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
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.

None yet

5 participants