Repository navigation
Add Density API to ThemeData, implement for buttons. - #43547
Conversation
d9ad0f6 to
ea22eb1
Compare
HansMuller
left a comment
There was a problem hiding this comment.
Generally speaking this looks fine. A few overall concerns though:
-
Should we give Density a less generic name? VisualDensity maybe? It's possible that "Density" will collide with an existing name in some apps.
-
It might be best to leave density out of ButtonTheme, and all the rest of the component themes moving forward. At least for now. ButtonTheme includes a colorScheme parameter, which was probably a mistake. Most of the other component themes do not provide properties that shadow generic theme parameters like textTheme or primaryColor. Similarly, specifying density per component also seems like it might be overdoing it, at least at the outset.
-
Prefer a gallery demo in a separate PR to a manual test.
Sure. I like
OK, I'll remove it from
Actually, I don't think the manual test belongs in the gallery. I mean, there should be a gallery demo for density too (probably just a setting in the overall settings page), but this manual test is really useful as a separate app. I wrote it because it allows me to take a single component and put it under manual testing where I can actually see it, trying it out in a variety of conditions so that I can quickly see how it behaves under odd conditions that I don't think we want to duplicate in the gallery app. It's also a lot smaller when I have to debug something and look at the widget tree. For different components, both Darren and I have independently invented similar apps to do what this app does, and it seemed inefficient to keep re-inventing it. My hope is that these will proliferate, and we'll end up with an easy way to boot up any component and twist the dials and see what happens. I'm happy to put it into another PR, of course. |
40cc0de to
19f74d3
Compare
a730bf3 to
cb3fef2
Compare
|
@HansMuller and I talked about this a bit more, and arrived at having a "profile" on the For particular controls, it would adjust based on deltas from the spec instead of using the absolute spec value. So, for instance, if the spec says that for the "comfortable" profile an And, I implemented it here, with It works OK, but there is a problem... you can't lerp an enum, so it blows our ability to lerp smoothly between two "profiles". Sadness. |
cb3fef2 to
c5d2eca
Compare
|
I think the best thing now might to just rip out the profile stuff entirely, and just associate numbers with the profile names (set up static doubles for the profile types, so |
e877330 to
2bbf89e
Compare
7768637 to
9cdfd7d
Compare
b3ff725 to
3546f75
Compare
Description
This implements a draft API for specifying the concept of
VisualDensityfor a Material user interface. This will allow us to specify default densities for desktop platforms that are more reasonable, since desktop platforms don't have the same constraints around tap target sizes.This PR adds the new
VisualDensityclass, and an attribute on theThemeDataandButtonThemeDataclasses for setting it. I applied this new theme information to the buttons which useRawMaterialButton. The implementation of the density changes is inRawMaterialButton.Tests
Breaking Change