Improve FHIR capture accessibility and recording feedback#3
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdded focus-visible and .sr-only utilities to the Tailwind theme; relocated a Google Fonts import. Reworked the Home.razor to a 4-step, accessible recording/extraction workflow with pause/resume and status reporting. Extended the browser audio recorder with pause/resume, AudioContext lifecycle, and tone cues. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Home as Home Component
participant Recorder as Audio Recorder
participant API as Backend/API
User->>Home: Click "Start"
Home->>Recorder: start()
Recorder->>Recorder: getAudioContext() / begin capture
Recorder->>Recorder: playCue("start")
User->>Home: Click "Pause"
Home->>Recorder: pause()
Recorder->>Recorder: pause capture
Recorder->>Recorder: playCue("pause")
User->>Home: Click "Resume"
Home->>Recorder: resume()
Recorder->>Recorder: resume capture
Recorder->>Recorder: playCue("resume")
User->>Home: Click "Stop"
Home->>Recorder: stopAndProcess()
Recorder->>API: upload/process audio
API-->>Home: processing result
Home->>Home: advance workflow step / render results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Summary of ChangesHello @venikman, 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 user experience and accessibility of the FHIR capture application. It focuses on making the audio recording and data extraction workflow more robust, user-friendly, and compliant with accessibility standards. Key improvements include better feedback mechanisms for recording status, clearer validation for user inputs, and a more navigable interface, all contributing to a smoother and more informative interaction for users capturing patient concerns. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
Pull request overview
This pull request enhances the FHIR capture interface with comprehensive accessibility improvements, recording controls, and user feedback mechanisms.
Changes:
- Added pause/resume controls for audio recording with audio cues and visual status indicators
- Implemented accessibility features including focus-visible styles, ARIA attributes, screen reader announcements, and validation messages
- Enhanced UI with improved contrast (ink-500 to ink-700), progress tracking, and state-specific status panels
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/AudioSharp.App/wwwroot/js/audio-recorder.js | Adds pause/resume recording functions, audio context management, and tone-based audio cues for recording state changes |
| src/AudioSharp.App/wwwroot/app.css | Compiled CSS with new utilities for accessibility (sr-only, focus-visible), status states (paused, error variants), and improved spacing |
| packages/tailwind-theme/theme.css | Defines focus-visible outline styles and sr-only utility class for screen reader support |
| src/AudioSharp.App/Components/Pages/Home.razor | Implements pause/resume UI, progress bar with workflow tracking, validation messages, ARIA attributes, screen reader announcements, and enhanced status panel with role-based styling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant number of improvements to the FHIR capture page, focusing on accessibility and user feedback. The changes are well-implemented and cover a wide range of enhancements, from focus styles and screen reader support to a more robust recording and status feedback system. The addition of pause/resume functionality, audio cues, and detailed validation messages will greatly improve the user experience. The code is well-structured, particularly the state management in the Blazor component. I have a couple of suggestions: one to modernize a CSS utility and another to fix a potential race condition in the JavaScript for playing audio cues. Overall, this is an excellent set of changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/AudioSharp.App/wwwroot/js/audio-recorder.js`:
- Around line 247-249: The code currently calls context.resume() without
awaiting it, which can let subsequent playTone calls run before the AudioContext
is active; modify the playCue function to be async and await context.resume()
when context.state === "suspended", then proceed to call playTone only after the
await; ensure any callers (e.g., the Blazor invoke from Home.razor via
InvokeVoidAsync) still work with the now-async playCue.
🧹 Nitpick comments (4)
packages/tailwind-theme/theme.css (1)
114-124: Consider usingclip-pathinstead of deprecatedclipproperty.The
clipproperty (line 121) is deprecated in CSS. While it still works for backwards compatibility, the modern approach usesclip-path: inset(50%).♻️ Suggested update
.sr-only { position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; - clip: rect(0, 0, 0, 0); + clip-path: inset(50%); white-space: nowrap; border-width: 0; }src/AudioSharp.App/wwwroot/js/audio-recorder.js (1)
221-230: Consider closingAudioContextduring cleanup to release resources.The
audioContextis lazily created but never closed. While modern browsers handle this reasonably well, explicitly closing it incleanup()would be cleaner, especially if the user performs multiple recording sessions.♻️ Suggested enhancement
const cleanup = () => { if (stream) { stream.getTracks().forEach((track) => track.stop()); } if (recorder) { recorder.destroy(); } + if (audioContext) { + audioContext.close().catch(() => {}); + audioContext = null; + } + recorder = null; stream = null; isRecording = false; isPaused = false; };src/AudioSharp.App/Components/Pages/Home.razor (2)
952-962: Empty catch block is acceptable here but consider logging.The comment explains that audio cue failures shouldn't block the capture flow, which is reasonable. However, silently swallowing exceptions can make debugging difficult if cues consistently fail.
♻️ Consider logging for diagnostics
private async Task PlayStatusCueAsync(string cue) { try { await JsRuntime.InvokeVoidAsync("audioRecorder.playCue", cue); } - catch + catch (Exception ex) { - // Ignore audio cue failures to avoid blocking capture flow. + // Log but don't block capture flow on audio cue failures. + System.Diagnostics.Debug.WriteLine($"Audio cue '{cue}' failed: {ex.Message}"); } }
412-416: Potential division by zero ifTotalStepsbecomes configurable.Currently
TotalStepsis hardcoded to4, so this is safe. However, ifTotalStepsever becomes0(e.g., from configuration),ProgressPercentwould throw aDivideByZeroException.♻️ Defensive approach
private int ProgressPercent => - (int)Math.Round(CurrentStep / (double)TotalSteps * 100, MidpointRounding.AwayFromZero); + TotalSteps > 0 + ? (int)Math.Round(CurrentStep / (double)TotalSteps * 100, MidpointRounding.AwayFromZero) + : 0;
|
Addressed review notes: playCue is now async and awaits AudioContext resume; sr-only uses clip-path; PlayStatusCueAsync logs failures; ProgressPercent guarded for TotalSteps==0; audioContext is closed during cleanup. CSS rebuilt. |
Summary\n- add focus-visible styles, screen reader utilities, and stronger TOC current-step styling\n- enhance recording controls with pause/resume, visual status panel, and audio cues\n- add validation/error UI for upload/transcript and improved FHIR send status messaging\n- improve secondary text contrast and adjust transcript sizing\n\n## Testing\n- npm run build:css
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.