-
Notifications
You must be signed in to change notification settings - Fork 545
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
[Android] Verify correct behavior of bleed property #2700
Conversation
marginLeft = -padding; | ||
} | ||
|
||
if(bleedDirection == ContainerBleedDirection.BleedToTrailing || bleedDirection == ContainerBleedDirection.BleedToBothEdges) |
There was a problem hiding this comment.
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
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. |
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) |
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) |
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. |
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. |
|
||
public static void ApplyPadding(ContainerStyle elementContainerStyle, ContainerStyle parentContainerStyle, LinearLayout collectionElementView, Context context, HostConfig hostConfig) | ||
{ | ||
if (elementContainerStyle != parentContainerStyle) |
There was a problem hiding this comment.
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.
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
Column.Bleed
ColumnSet.Bleed
Container.Bleed