Theme DaxIconButton colors and add filled variant#8823
Conversation
GerardPaligot
force-pushed
the
feature/gerard/compose-icon-button
branch
from
a56f03f to
8430706
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8430706. Configure here.
GerardPaligot
force-pushed
the
feature/gerard/compose-icon-button
branch
from
8430706 to
034c083
Compare
| Column { | ||
| DaxIconButton( | ||
| onClick = {}, | ||
| iconPainter = painterResource(R.drawable.ic_settings_24), | ||
| contentDescription = "Settings", | ||
| ) | ||
| DaxIconButton( | ||
| onClick = {}, | ||
| iconPainter = painterResource(R.drawable.ic_settings_24), | ||
| contentDescription = "Settings", | ||
| colors = DaxIconButtonDefaults.filledColors, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🔍 With these changes I no longer see the settings icon in the preview. I believe this related to how we tint some Icons, and which is why I went with Color.Unspecified, so that we default to the color in some our our VectorDrawables
There was a problem hiding this comment.
The icon wasn't displayed before my changes too. I've improved our PreviewBox component to configure ContextThemeWrapper and be able to evaluate XML attributes, but we should consider to move this in the DuckDuckGoTheme component to be compatible with interoperability between Compose and XML.
| * @param colors Container and content colors; defaults to [DaxIconButtonDefaults.colors]. | ||
| * @param interactionSource The interaction source for this button. | ||
| * | ||
| * Asana task: https://app.asana.com/1/137249556945/project/1202857801505092/task/1215540472063931?focus=true |
There was a problem hiding this comment.
❓Do we want to add this to ADS when it doesn't exist in Figma? If we think it's worthwhile we should propose to be added I guess/
There was a problem hiding this comment.
I think IconButton is important enough to be added in Figma, what do you think?
|
|
||
| @PreviewLightDark | ||
| @Composable | ||
| private fun DaxIconButtonPreview() { |
There was a problem hiding this comment.
🔍 I think it would be worthwhile adding a filled+disabled preview as well 👍
GerardPaligot
force-pushed
the
feature/gerard/compose-icon-button
branch
from
034c083 to
42a17ae
Compare
Tint the Compose icon button with the theme content color so icons follow light/dark mode instead of the drawable's baked-in colors. Add a filled container color variant, and show default, filled, and disabled states in the design system screen.
- Replace Material IconButtonColors with a DaxIconButtonColors data class, mapped to Material3 inside DaxIconButton (consistent with DaxButton). - Add a filled + disabled icon button preview. - PreviewBox applies the DuckDuckGo View theme to LocalContext so painterResource resolves drawable ?attr/daxColor* fills; previews otherwise rendered those vectors transparent.
GerardPaligot
force-pushed
the
feature/gerard/compose-icon-button
branch
from
42a17ae to
2e2afe7
Compare
2 participants
Task/Issue URL: https://app.asana.com/1/137249556945/project/1215496415658080/task/1215564036631797?focus=true
Description
Makes the Compose
DaxIconButtonfollow the Compose theme, and adds a filled variant.DuckDuckGoTheme.colors.icons.primaryby default) instead ofColor.Unspecified. Previously, icons with hard-coded fills (e.g.ic_ai_chat_24_solid_color, which is@color/black) rendered the wrong color in dark mode because the drawable's baked-in color was used instead of the Compose theme.enabledandcolorsparameters plusDaxIconButtonDefaults(colors= transparent container,filledColors= filled container).Steps to test this PR
Design system screen
UI changes
Note
Medium Risk
Default icon tinting changes appearance for every existing DaxIconButton call site; behavior is intentional but worth verifying screens that depended on native drawable colors.
Overview
DaxIconButtonnow applies Compose theme colors instead of leaving icons untinted (Color.Unspecified). Icons use the button’s content color (defaultDuckDuckGoTheme.colors.icons.primary) so light/dark and disabled states stay correct when drawables have baked-in fills.The API adds
enabled, acolorsparameter, andDaxIconButtonDefaultswith transparentcolorsand afilledColorsvariant (container + disabled container from theme backgrounds). Previews and the internal Components → Buttons screen show default, filled, and disabled examples.PreviewBoxwraps previewLocalContextinContextThemeWrapper(light/dark DuckDuckGo themes) sopainterResourcecan resolve?attr/drawable colors in Studio previews.Reviewed by Cursor Bugbot for commit 2e2afe7. Bugbot is set up for automated code reviews on this repo. Configure here.