refactor(site): demui <NotificationEvents /> into dropdown - #27732
Conversation
Documentation CheckUpdates Needed
No other docs changes appear needed: this is a presentation-only refactor with no API, CLI, or configuration surface changes. Automated review via Coder Agents |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80e355862e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| })} | ||
| </ToggleButtonGroup> | ||
| <SelectTrigger | ||
| aria-label="Notification method" |
There was a problem hiding this comment.
Include the template in each select's accessible name
For screen-reader users navigating the comboboxes directly, every template exposes the identical accessible name Notification method; the visible template name is only sibling text and is not referenced by the trigger, so users cannot tell which event they are modifying. Associate the trigger with tpl.name, for example through aria-labelledby or a template-specific label.
AGENTS.md reference: site/AGENTS.md:L18-L19
Useful? React with 👍 / 👎.
DanielleMaywood
left a comment
There was a problem hiding this comment.
Dropdown makes so much more sense here, brilliant
Note
We don't have a planned migration path for the MUI
<ToggleGroup />. Therefore, I've swapped these to dropdowns as an inbetween.Migrate deployment notification event settings off MUI
Card/List/ToggleButtonGroupand Emotion styles.Template groups now use the same Tailwind card layout as user notification settings, and the delivery method control is a shared
Selectdropdown with icon + label.