Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf223bc9c5
ℹ️ 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".
| className="bg-surface rounded-xl border border-border w-full max-w-lg flex flex-col shadow-xl" | ||
| style={{ height: 'min(80vh, 600px)' }} | ||
| onClick={(e) => e.stopPropagation()} | ||
| onKeyDown={(e) => e.stopPropagation()} |
There was a problem hiding this comment.
The new onKeyDown={(e) => e.stopPropagation()} on the inner modal container prevents Escape from reaching the outer overlay handler (onKeyDown={(e) => e.key === 'Escape' && setOpen(false)}), so keyboard users focused inside the dialog cannot dismiss it with Escape. This makes the newly added Escape-close behavior effectively unreachable during normal use.
Useful? React with 👍 / 👎.
| <div | ||
| className="bg-surface border border-border rounded-xl shadow-xl w-full max-w-lg max-h-[70vh] flex flex-col" | ||
| onClick={(e) => e.stopPropagation()} | ||
| onKeyDown={(e) => e.stopPropagation()} |
There was a problem hiding this comment.
Stopping all keydown propagation on the inner project manager modal blocks the overlay's Escape handler, so pressing Escape while focus is in any control inside the modal will not close it. Since the close logic was added on the outer container in this change, this child-level stopPropagation defeats that behavior.
Useful? React with 👍 / 👎.
| <div | ||
| className="bg-surface rounded-xl border border-border w-full max-w-md flex flex-col shadow-xl p-6" | ||
| onClick={(e) => e.stopPropagation()} | ||
| onKeyDown={(e) => e.stopPropagation()} |
There was a problem hiding this comment.
The inner custom provider modal now stops all keydown propagation, which prevents the outer overlay's Escape handler from firing when focus is inside the form. As a result, users cannot dismiss this modal with Escape during typical keyboard interaction.
Useful? React with 👍 / 👎.
| onClick={(e) => e.stopPropagation()} | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="confirm-dialog-title" |
There was a problem hiding this comment.
This dialog now sets aria-labelledby="confirm-dialog-title", but there is no element in the dialog with that ID, so assistive tech cannot resolve the label reference. The dialog therefore loses its explicit accessible name and will fail ARIA ID-reference validation.
Useful? React with 👍 / 👎.
|
1 participant
No description provided.