-
Notifications
You must be signed in to change notification settings - Fork 98
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
[Tiles] Split LayoutPreview api into element/root variants. #240
[Tiles] Split LayoutPreview api into element/root variants. #240
Conversation
layout: LayoutElement, | ||
public fun LayoutElementPreview( | ||
element: LayoutElement, | ||
@ColorInt windowBackgroundColor: Int = Color.DKGRAY, |
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.
Reverted this to DKGRAY where in the issue it was BLACK.
@@ -163,7 +163,7 @@ fun SampleButtonImagePreview() { | |||
.setId("click") | |||
.build() | |||
|
|||
LayoutPreview( | |||
LayoutElementPreview( |
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.
So previously, LayoutPreview
was only suitable for small components like buttons. The equivalent new API is LayoutElementPreview
.
The instrumentation tests can be flaky, If they are mostly passing then just land it. |
layout: LayoutElement, | ||
public fun LayoutElementPreview( | ||
element: LayoutElement, | ||
@ColorInt windowBackgroundColor: Int = Color.DKGRAY, |
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.
I wonder if we should actually prefer Compose colors here in the API? Thoughts? Can be a follow up if so.
WHAT
Splits the
LayoutPreview
preview API for tiles intoLayoutElementPreview
andLayoutRootPreview
.WHY
The previous LayoutPreview API only worked for non-root elements, like Button. This is because the passed LayoutElement was also wrapped in a BoxLayout so that the passed element could be rendered in the center of the screen. Passing a root element (like PrimaryLayout) to this API which assumes that it has full width and height didn't work.
HOW
This PR adds two APIs, one intended for elements and one for root layouts. They're chained together, such that the element one will wrap the element in a BoxLayout (a root layout), and pass that to the root layout API.
Checklist 📋
[-] Add explicit visibility modifier and explicit return types for public declarations
[-] Run spotless check
[] Run tests
[-] Update metalava's signature text files