-
Notifications
You must be signed in to change notification settings - Fork 50
QE Top section updates #664
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
Conversation
📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
|
dd03a72 to
f12f8a0
Compare
f12f8a0 to
6132b3b
Compare
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.
🚀 Thanks for the changes! I left a minor question to discuss.
✅ Launch the QE with and without the token - confirm the email visibility.
✅ Test the new icon in RTL - it should be mirrored.
✅ Test the font scaling and confirm it stops at a certain threshold.
| state = state, | ||
| modifier = Modifier.height(32.dp), | ||
| ) { | ||
| Icon( |
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 (probably not now) we should replace the default icon in the UI library with this new icon.
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 believe iOS doesn't use the public component, so the new icon was changed only in the QE. That's why I didn't modify the public component as well. The icon can be easily modified (as we did here) so I don't think it's a big issue.
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.
Yeah, it makes total sense for this PR. I was thinking aloud. I agree that it's not a big issue.
Description
This PR makes some changes to the QE top bar and the profile card:
View buttonis changed to "external link".Screen_recording_20250616_094128.mp4
Testing Steps