🎨 Palette: Improve Create Song Dialog UX and Accessibility#37
🎨 Palette: Improve Create Song Dialog UX and Accessibility#37imLeGEnDco55 wants to merge 1 commit into
Conversation
- Add clear button to file input in CreateSongDialog - Add aria-labels to SongEditor buttons and inputs - Improve accessibility of hidden file input with htmlFor/id association - Add test for CreateSongDialog clear functionality Co-authored-by: imLeGEnDco55 <210333019+imLeGEnDco55@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @imLeGEnDco55, 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 enhances the user experience and accessibility of the Create Song Dialog and Song Editor components. It introduces a clear button for the file input in the Create Song Dialog, resolving the issue of users being unable to easily remove a selected file. Additionally, it improves accessibility by adding 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.
🟡 File input value not reset on submit/close, preventing re-selection of same file
When the dialog is submitted via handleSubmit or closed via handleClose, setAudioFile(null) is called but fileInputRef.current.value is never reset to ''. The new handleClearFile at line 32-37 correctly resets both, establishing the pattern, but handleSubmit and handleClose omit the file input reset.
Detailed Explanation & Impact
After a user selects a file (e.g., song.mp3), submits the form, and then reopens the dialog to create another song:
- The
audioFilestate isnull(correctly reset), so the "Seleccionar archivo de audio" button is shown. - The user clicks the button and selects the same
song.mp3file. - The browser's
<input type="file">does NOT fireonChangebecause its.valuestill holds the previous filename. - The file appears to not be selected — the user sees no feedback.
The same issue occurs when the dialog is closed via handleClose (line 60-64). The handleClearFile function (line 32-37) correctly resets fileInputRef.current.value = '', but this pattern was not applied to handleSubmit (line 50-51) or handleClose (line 61-62).
Impact: Users cannot re-select the same audio file across consecutive dialog sessions without first selecting a different file.
(Refers to lines 49-52)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Code Review
This pull request enhances the user experience and accessibility of the CreateSongDialog and SongEditor components. The addition of a clear button for the file input is a great UX improvement, and the new aria-label attributes on various elements significantly improve accessibility. The implementation is solid, and I have one suggestion for CreateSongDialog.tsx to refactor the file reset logic for better robustness and reusability, which will address a potential edge-case bug.
| const handleClearFile = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| setAudioFile(null); | ||
| if (fileInputRef.current) { | ||
| fileInputRef.current.value = ''; | ||
| } | ||
| }; |
There was a problem hiding this comment.
To improve code reusability and ensure consistent state management, it's a good practice to extract the logic for resetting the audio file into a dedicated function. This function can then be used here in handleClearFile, and also in other places like handleClose and handleSubmit where the form is reset. This will prevent a subtle bug where re-selecting the same file after closing/submitting the dialog doesn't work because the underlying file input's value hasn't been cleared.
const resetAudioFile = () => {
setAudioFile(null);
if (fileInputRef.current) {
fileInputRef.current.value = '';
}
};
const handleClearFile = (e: React.MouseEvent) => {
e.stopPropagation();
resetAudioFile();
};
💡 What: Added a clear button to the file input in the Create Song Dialog and added aria-labels to icon-only buttons in the Song Editor.
🎯 Why: Users could not easily remove a selected file in the Create Song Dialog without closing it. Icon-only buttons were inaccessible to screen readers.
📸 Before/After: The file input now shows the filename with an 'X' button to clear it.
♿ Accessibility: Added
aria-labelto Back, More Options, and Title inputs. Ensured hidden file input is properly labeled.PR created automatically by Jules for task 14973417009128847106 started by @imLeGEnDco55