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

[iOS] Replace the grid/list switching icon #535

Merged

Conversation

hicka04
Copy link
Contributor

@hicka04 hicka04 commented Aug 16, 2024

Issue

Overview (Required)

  • Replace the grid/list switching icon with Figma design

Links

Screenshot (Optional if screenshot test is present or unrelated to UI)

Before After

Movie (Optional)

Before After

Comment on lines +75 to +77
Image(switchModeIcon)
.foregroundStyle(AssetColors.Surface.onSurface.swiftUIColor)
.frame(width: 40, height: 40)
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem has not occurred because the original image color is onSurface, but I would expect that the color would not be changed unless the template is originally specified in the renderingMode modifier.

Suggested change
Image(switchModeIcon)
.foregroundStyle(AssetColors.Surface.onSurface.swiftUIColor)
.frame(width: 40, height: 40)
Image(switchModeIcon)
.renderingMode(.template)
.foregroundStyle(AssetColors.Surface.onSurface.swiftUIColor)
.frame(width: 40, height: 40)

I think it would be safer to add this one.
Alternatively, you can also set template-rendering-intent to imageset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shin-usu
Thanks your review!
I think these icons should always be template mode, so I set rendering mode on imageset.
40b52d5
Please review again 🙏

Copy link
Contributor

@shin-usu shin-usu left a comment

Choose a reason for hiding this comment

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

LGTM!! Thank you!

@shin-usu shin-usu merged commit 454b13c into DroidKaigi:main Aug 17, 2024
4 checks passed
@hicka04 hicka04 deleted the ios-replace-grid-list-switching-icon branch August 17, 2024 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Replace the grid/list switching icon
2 participants