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

Add new levels of notification for dev toolbar apps #10252

Merged
merged 11 commits into from Mar 8, 2024

Conversation

Princesseuh
Copy link
Member

@Princesseuh Princesseuh commented Feb 27, 2024

Changes

Some users asked for the ability to show a little pill that wasn't as threatening as the red dots we have right now. This adds that.

image

Testing

Added a test

Docs

Will do!

Copy link

changeset-bot bot commented Feb 27, 2024

🦋 Changeset detected

Latest commit: 6c936ec

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release labels Feb 27, 2024
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

@Princesseuh Princesseuh marked this pull request as ready for review February 27, 2024 17:04
@Princesseuh Princesseuh added this to the 4.5 milestone Feb 27, 2024
@ematipico
Copy link
Member

Nit: I would make the info level green or blue :)

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

This is exciting stuff! Nice work @Princesseuh 🚥

We should probably not rely only on colour for accessibility reasons — colour vision variations are really common. It's definitely a bit tricky at this scale though.

Let me know if it would be helpful for me to actually suggest something rather than just noting this!

@Princesseuh
Copy link
Member Author

This is exciting stuff! Nice work @Princesseuh 🚥

We should probably not rely only on colour for accessibility reasons — colour vision variations are really common. It's definitely a bit tricky at this scale though.

Let me know if it would be helpful for me to actually suggest something rather than just noting this!

Sure, feel free to suggest improvements! We definitely don't have a lot of room, so we can't really like, add visual text, but I'm sure we can figure something out

@natemoo-re
Copy link
Member

natemoo-re commented Feb 27, 2024

Great suggestion @delucis! @Princesseuh here's a concept for shape + color indicators.

Red circle for errors because that's what we already have. Yellow triangle for error because it's similar to a ⚠️ symbol. Blue square for info because it's similar to a ℹ️ symbol.

three icons in a black bar, the first has a red square to indicate an error, the second has a yellow triangle to indicate a warning, and the third has a blue circle to indicate info
type color SVG
error #B33E66 <svg viewBox="0 0 10 10"><rect width="9" height="9" x=".5" y=".5" fill="currentColor" stroke="#13151A" rx="4.5"/></svg>
warning #B58A2D <svg viewBox="0 0 12 10"><path fill="currentColor" stroke="#13151A" d="m10.76 7.25-3.46-6c-.58-1-2.02-1-2.6 0l-3.46 6a1.5 1.5 0 0 0 1.3 2.25h6.92a1.5 1.5 0 0 0 1.3-2.25Z"/></svg>
info #3645D9 <svg viewBox="0 0 10 10"><rect width="9" height="9" x=".5" y=".5" fill="currentColor" stroke="#13151A" rx="1.5"/></svg>

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Code changes look great, left a suggestion for a slightly updated UI to improve a11y.

@delucis
Copy link
Member

delucis commented Feb 29, 2024

+1 on Nate’s suggestion! This is pretty much what I was thinking too.

Not sure if the suggested blue has quite high enough contrast with the dark background? So exact colour shade might want checking, but for the general idea: two thumbs up!

(Aside: I hadn’t seen GitHub displaying the colour in hex codes before — that’s cool!)

Copy link
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Docs approval!

@Princesseuh Princesseuh merged commit 3307cb3 into main Mar 8, 2024
3 of 4 checks passed
@Princesseuh Princesseuh deleted the feat/all-notifications-toolbar branch March 8, 2024 10:53
@astrobot-houston astrobot-houston mentioned this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) semver: minor Change triggers a `minor` release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants