fix(ui): move click handlers to ButtonBase for keyboard accessibility#5821
fix(ui): move click handlers to ButtonBase for keyboard accessibility#5821eren-karakus0 wants to merge 2 commits intoFlowiseAI:mainfrom
Conversation
…ccessibility Move onClick event handlers from inner Avatar (div) elements to outer ButtonBase (button) elements in CanvasHeader toolbar buttons. This enables keyboard users to trigger button actions with Enter/Space keys, as div elements are not focusable by default. Fixes all 7 toolbar buttons: Back, Edit Name, Save Name, Cancel, API Endpoint, Save Chatflow, and Settings. Fixes FlowiseAI#5819
Summary of ChangesHello @eren-karakus0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the keyboard accessibility of the canvas editor's toolbar buttons by correctly associating their interactive behavior with the appropriate HTML elements. By relocating click handlers to the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an important accessibility issue by moving onClick handlers from non-interactive Avatar components to their parent ButtonBase components. This ensures that the toolbar buttons are keyboard-focusable and can be triggered with Enter or Space. The changes are applied consistently across all relevant buttons in CanvasHeader.jsx. I have one suggestion to improve code readability by extracting a multi-line event handler into a named function. Overall, this is a solid fix that improves the user experience for keyboard users.
| onClick={() => { | ||
| if (window.history.state && window.history.state.idx > 0) { | ||
| navigate(-1) | ||
| } else { | ||
| navigate('/', { replace: true }) | ||
| } | ||
| }} |
There was a problem hiding this comment.
For improved readability and to keep the JSX cleaner, you could extract this logic into a named function within the CanvasHeader component.
For example, you could define:
const handleBackClick = () => {
if (window.history.state && window.history.state.idx > 0) {
navigate(-1);
} else {
navigate('/', { replace: true });
}
};And then use onClick={handleBackClick} in the ButtonBase component. This is especially helpful for logic that isn't a simple one-liner and improves maintainability.
There was a problem hiding this comment.
Extracted it into a named handleBackClick function, consistent with the existing handler pattern in the component (submitFlowName, onAPIDialogClick, etc). Fixed in 5c58b66.
Extract the multi-line back navigation logic from inline onClick into a named handleBackClick function for improved readability, consistent with other handlers in the component (submitFlowName, onAPIDialogClick, etc).
|
Tested and It worked very well. Thank you very much. Also the other buttons should be repaired in the same way. I discovered other accesibility problems too, but I will make issues with them and kind of solution. Thank you very much for your openess to solve accesibility problems. |
|
Thank you for testing! Glad it works well. I noted the same pattern in other files (MarketplaceCanvasHeader, CustomAssistantConfigurePreview, EditNodeDialog, Header) - happy to submit follow-up PRs for those once this one is merged. Looking forward to the new issues you'll open, accessibility improvements are important. |
|
Friendly ping - this has been tested and confirmed working by the issue reporter. Just checking if there's anything else needed for review. Happy to address any feedback! |
Summary
Fixes #5819
The toolbar buttons in
CanvasHeader(Back,Edit Name,Save Name,Cancel,API Endpoint,Save Chatflow,Settings) had theironClickhandlers bound to the innerAvatarcomponent, which renders as a<div>. Since<div>elements are not keyboard-focusable by default, pressingEnterorSpaceon a focusedButtonBasedid not trigger the click handler.This PR moves all
onClickhandlers from the innerAvatarto the outerButtonBasecomponent, which renders as a<button>element and is natively keyboard-interactive.Changes
packages/ui/src/views/canvas/CanvasHeader.jsxonClickfromAvatartoButtonBasefor all 7 toolbar buttonsHow to test
Tabkey to navigate to toolbar buttons (Back, Save, Settings, etc.)EnterorSpace- the button action should now trigger