-
Notifications
You must be signed in to change notification settings - Fork 356
Convert ComposerAlertMolecule to use alert levels.
#5691
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
Convert ComposerAlertMolecule to use alert levels.
#5691
Conversation
|
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
...n/kotlin/io/element/android/libraries/designsystem/atomic/molecules/ComposerAlertMolecule.kt
Outdated
Show resolved
Hide resolved
bmarty
left a comment
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.
The code looks good, but it seems that for some reason (probably because you are using a fork) the screenshot cannot be recorded.
If the PR is ready to be merged (i.e. not a draft anymore), I can merge it in a branch of me and record the screenshot from there.
...n/kotlin/io/element/android/libraries/designsystem/atomic/molecules/ComposerAlertMolecule.kt
Show resolved
Hide resolved
I suspect so; based on what I gleaned from the logs, my fork is missing a Maestro access token. This is ready, so I'll mark this as ready for review, and we can go from there! |
|
I'm slightly worried the dark theme info alert doesn't have good contrast - I had similar grievances on element-hq/element-web#31156, which I believe is a question for the design team. |
bmarty
left a comment
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 update. One final comment, then I'll merge your branch to mine to be able to record the screenshots.
39aa793
into
element-hq:feature/bma/improveComposerAlertMolecule
Content
Introduces
ComposerAlertLeveland modifiesComposerAlertMoleculeto accept alevelargument overisCritical. Displays an icon appropriate for the level if no avatar is specified.Motivation and context
The designs for UX as part of element-hq/element-meta#2875 call for an alert using the
infocolour palette, which the current component doesn't support.Screenshots / GIFs
Tests
The
ComposerAlertLevel.Criticalis identical to previous iterations:This was tested using the identity reset alert mechanism.
Tested devices
Checklist