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. 📝 WalkthroughWalkthroughAdds a new Tailwind theme package (Noto Sans, tokens, utilities), consolidates app styling to import the theme, refactors layout/navigation to header-based responsive design, extracts telemetry UI into a new TelemetryPanel component, restructures Home into a responsive grid, and removes an inline Fast components script. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🪛 Biome (2.1.2)src/AudioSharp.App/wwwroot/app.css[error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally. -webkit-text-decoration is already defined here. Remove or rename the duplicate property to ensure consistent styling. (lint/suspicious/noDuplicateProperties) [error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally. -webkit-text-decoration-color is already defined here. Remove or rename the duplicate property to ensure consistent styling. (lint/suspicious/noDuplicateProperties) [error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally. -webkit-text-decoration-color is already defined here. Remove or rename the duplicate property to ensure consistent styling. (lint/suspicious/noDuplicateProperties) 🔇 Additional comments (15)
✏️ Tip: You can disable this entire section by setting 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 refactors the application's styling by introducing a new, centralized Tailwind theme package. The primary goal is to modernize the UI, improve light-mode readability, and remove legacy styling dependencies, leading to a more consistent and maintainable visual experience across the application. 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
|
![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 and positive refresh of the UI by creating a shared Tailwind theme package. This centralizes styling, removes legacy assets, and improves code maintainability. The new layout and component structure are well-executed. However, I've identified a critical issue in the theme's implementation where an aggressive border-radius reset breaks Tailwind's rounding utilities, which are used throughout the new UI components. I've also included a few other suggestions to improve the theme package and fix a minor visual bug in an animation.
| .rounded, | ||
| .rounded-sm, | ||
| .rounded-md, | ||
| .rounded-lg, | ||
| .rounded-xl, | ||
| .rounded-2xl, | ||
| .rounded-3xl, | ||
| .rounded-full, | ||
| .rounded-card, | ||
| .file\:rounded-full::file-selector-button { | ||
| border-radius: 0; | ||
| } |
There was a problem hiding this comment.
These overrides force all Tailwind rounded-* utility classes to have a border-radius of 0. While the goal is a "no-radius default", this implementation is too aggressive and makes it impossible to use rounded corners with Tailwind's utility classes. Several components in this PR use classes like rounded-2xl, rounded-full, and rounded-card (e.g., in MainLayout.razor, TelemetryPanel.razor, Home.razor), but this override will render them ineffective, which is likely not the intention.
A better approach for a "no-radius default" is to rely on the CSS variables for radius that are already defined for shadcn compatibility (lines 56-62). Components can use these variables, and they can be overridden when needed.
Please remove this entire block of overrides to allow Tailwind's rounded-* utilities to work as expected.
| { | ||
| "name": "@audiosharp/tailwind-theme", | ||
| "version": "0.1.0", | ||
| "private": true, |
There was a problem hiding this comment.
The package is marked as private, which prevents it from being published to an npm registry. The README.md file mentions publishing as a usage option. If this package is intended to be published, "private": true should be removed or set to false.
| "private": true, | |
| "private": false, |
| :root, | ||
| .dark { |
There was a problem hiding this comment.
The .dark selector is included with :root and is being assigned light theme variables, including color-scheme: light. This will cause incorrect styling if a dark theme is ever introduced, and it forces light-mode native UI elements (like scrollbars) even when the .dark class is applied. It's better to remove .dark from this rule-set for now to avoid confusion and future bugs. A separate .dark block should be created when the dark theme is actually implemented.
:root {| border: 3px solid #0087ff; | ||
| opacity: 1; | ||
| border-radius: 50%; | ||
| border-radius: 0; |
There was a problem hiding this comment.
Changing border-radius to 0 for the .components-rejoining-animation div element will change the reconnect animation from an expanding circle to an expanding square. This is likely an unintended visual change. To preserve the circular ripple effect, this should be restored to 50%.
| border-radius: 0; | |
| border-radius: 50%; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/AudioSharp.App/Components/Pages/Home.razor (1)
763-770: Potential reentrancy when auto-stopping recording.When
RecordingSeconds >= MaxRecordingSeconds, the timer callsStopRecordingAsyncviaInvokeAsync. If the user simultaneously clicks "Stop & analyze", both paths could race to stop the recording. Consider adding a guard or using a lock-free check.Suggested guard
if (RecordingSeconds >= MaxRecordingSeconds) { - await InvokeAsync(StopRecordingAsync); + await InvokeAsync(async () => + { + if (IsRecording) + { + await StopRecordingAsync(); + } + }); return; }Note:
StopRecordingAsyncalready has an early return if!IsRecording, so this is defense-in-depth rather than a critical fix.
🧹 Nitpick comments (10)
packages/tailwind-theme/theme.css (3)
4-4: Consider using a proper monospace font for--font-mono.Using "Noto Sans" (a proportional sans-serif) for
--font-monois unconventional. Monospace fonts should use fixed-width typefaces for proper code/preformatted text display.Suggested fix
- --font-mono: "Noto Sans", "Segoe UI", system-ui, sans-serif; + --font-mono: ui-monospace, SFMono-Regular, "SF Mono", Menlo, Consolas, "Liberation Mono", monospace;
34-36: The.darkclass applies light-mode styles, which may cause confusion.Both
:rootand.darkshare identical light-mode values withcolor-scheme: light. This means toggling the.darkclass has no visual effect. If dark mode is intentionally unsupported, consider removing the.darkselector to avoid misleading future maintainers. If dark mode support is planned, the.darkblock should define dark-mode overrides.
87-87:filter: blur(0px)is a no-op.This declaration has no visual effect. If blur was intended for the decorative blobs, consider a non-zero value. Otherwise, this line can be removed.
Suggested fix
- filter: blur(0px); + /* Remove or add actual blur value if decorative blur is desired */src/AudioSharp.App/Components/Layout/ReconnectModal.razor.css (1)
91-105: Inconsistent button state colors.The default button background uses the new CSS variable
var(--color-brand-500), but:hoverand:activestates still use hardcoded blue colors (#3b6ea2,#6b9ed2) that don't align with the amber brand tokens mentioned in the PR objectives.♻️ Suggested fix
`#components-reconnect-modal` button:hover { - background-color: `#3b6ea2`; + background-color: var(--color-brand-600, `#d97706`); } `#components-reconnect-modal` button:active { - background-color: `#6b9ed2`; + background-color: var(--color-brand-400, `#fbbf24`); }src/AudioSharp.App/Components/Layout/MainLayout.razor (1)
12-20: Consider accessibility for the dismiss control.The dismiss span uses an emoji (🗙) without semantic meaning. For screen readers and keyboard navigation, consider:
- Adding
role="button"andaria-label="Dismiss error"- Adding
tabindex="0"for keyboard accessibility♻️ Suggested improvement
- <span class="dismiss ml-auto cursor-pointer text-brand-300">🗙</span> + <span class="dismiss ml-auto cursor-pointer text-brand-300" + role="button" + aria-label="Dismiss error" + tabindex="0">🗙</span>src/AudioSharp.App/Components/Layout/TelemetryPanel.razor (2)
68-73: Fire-and-forget pattern may swallow exceptions.The
_ = InvokeAsync(StateHasChanged)discards the Task, which means any exceptions thrown during rendering won't be observed. Consider adding error handling or usingConfigureAwait(false)with proper exception logging.♻️ Suggested improvement
private void OnTelemetryChanged() { Snapshot = AiUsageTelemetry.GetSnapshot(); RecentEvents = AiUsageTelemetry.GetRecentEvents(); - _ = InvokeAsync(StateHasChanged); + _ = InvokeAsync(StateHasChanged).ContinueWith(t => + { + if (t.IsFaulted) + { + Console.Error.WriteLine($"TelemetryPanel render error: {t.Exception?.InnerException?.Message}"); + } + }, TaskContinuationOptions.OnlyOnFaulted); }Alternatively, if the
Changedevent can be made async-aware, consider making the handler async.
47-53: Consider extracting the conditional class logic.The inline conditional class string is getting long. For readability, consider extracting to a helper method.
♻️ Suggested improvement
private static string GetEventClasses(AiTelemetryEvent telemetryEvent) { var baseClasses = "mt-2 rounded-lg border border-black/20 bg-surface-soft px-3 py-2 text-[12px] text-ink-700 border-l-4"; return telemetryEvent.Success ? $"{baseClasses} border-l-emerald-400" : $"{baseClasses} border-l-rose-400"; }Then in markup:
<div class="@GetEventClasses(telemetryEvent)">src/AudioSharp.App/Components/Layout/NavMenu.razor (1)
15-23: Consider mobile menu behavior.The
<details>element doesn't automatically close when:
- A link is clicked (navigating to an anchor on the same page)
- User clicks outside the menu
This is standard browser behavior but may feel unexpected to users. Consider adding JS to close the menu on link click or outside click, or document this as a known limitation.
If you want to auto-close on link click, you could add an
onclickhandler:<a class="block py-1 transition hover:text-white" href="#capture" `@onclick`="() => { /* close details via JS interop */ }">Capture</a>Or use a simple inline approach if the parent details element has an id.
src/AudioSharp.App/Components/Pages/Home.razor (2)
687-716: Consider using a cancellable token for the FHIR upload.
SendToFhirAsyncpassesCancellationToken.NonetoUploadBundleAsync, which means the HTTP request cannot be cancelled if the user navigates away or the component is disposed. This could leave orphaned requests in flight.Suggested improvement
private async Task SendToFhirAsync() { if (!CanSendToFhir) { return; } + ResetProcessing(); IsSendingToFhir = true; FhirStatusMessage = "Sending bundle to FHIR server..."; try { var result = await FhirServerClient - .UploadBundleAsync(BundleJson, CancellationToken.None); + .UploadBundleAsync(BundleJson, ProcessingToken); FhirStatusMessage = result.Success ? (string.IsNullOrWhiteSpace(result.Location) ? "FHIR server accepted the bundle." : $"FHIR server accepted the bundle: {result.Location}") : $"FHIR server returned {result.StatusCode}."; } + catch (OperationCanceledException) + { + FhirStatusMessage = "FHIR send canceled."; + } catch (Exception ex) { FhirStatusMessage = $"FHIR send failed: {ex.Message}"; } finally { IsSendingToFhir = false; } }
528-557: Consider using a cancellable token for the save operation.Similar to the FHIR upload,
SaveAsyncusesCancellationToken.Nonefor the repository call. While save operations are typically quick, using a cancellable token maintains consistency and allows cleanup on component disposal.Suggested improvement
- await ConcernRepository.AddAsync(record, CancellationToken.None); + await ConcernRepository.AddAsync(record, ProcessingToken);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/AudioSharp.App/wwwroot/lib/fast/fast-components.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (14)
packages/tailwind-theme/README.mdpackages/tailwind-theme/fonts.csspackages/tailwind-theme/package.jsonpackages/tailwind-theme/theme.csssrc/AudioSharp.App/Components/App.razorsrc/AudioSharp.App/Components/Layout/MainLayout.razorsrc/AudioSharp.App/Components/Layout/MainLayout.razor.csssrc/AudioSharp.App/Components/Layout/NavMenu.razorsrc/AudioSharp.App/Components/Layout/NavMenu.razor.csssrc/AudioSharp.App/Components/Layout/ReconnectModal.razor.csssrc/AudioSharp.App/Components/Layout/TelemetryPanel.razorsrc/AudioSharp.App/Components/Pages/Home.razorsrc/AudioSharp.App/Styles/app.csssrc/AudioSharp.App/wwwroot/app.css
💤 Files with no reviewable changes (3)
- src/AudioSharp.App/Components/Layout/MainLayout.razor.css
- src/AudioSharp.App/Components/Layout/NavMenu.razor.css
- src/AudioSharp.App/Components/App.razor
🧰 Additional context used
🪛 Biome (2.1.2)
src/AudioSharp.App/wwwroot/app.css
[error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
-webkit-text-decoration is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
-webkit-text-decoration-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
[error] 2-2: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
-webkit-text-decoration-color is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (15)
packages/tailwind-theme/fonts.css (1)
1-1: LGTM! Good font loading configuration.The
display=swapparameter is a best practice that prevents Flash of Invisible Text (FOIT). The selected font weights (400-700) provide good typographic flexibility.One consideration: loading fonts from Google Fonts CDN introduces an external dependency and may have privacy implications (Google can track font requests). For applications with strict privacy requirements, consider self-hosting the font files.
src/AudioSharp.App/wwwroot/app.css (1)
1-2: Generated Tailwind output looks correct.This is compiled CSS output from Tailwind v4.1.18. The duplicate
-webkit-text-decorationproperties flagged by static analysis are intentional vendor prefix fallbacks generated by Tailwind for cross-browser compatibility—this is expected behavior for generated CSS and not a concern.Ensure this file is not manually edited; changes should be made to the source CSS files (
fonts.css,theme.css) and rebuilt vianpm run build:css.packages/tailwind-theme/theme.css (1)
158-163: Good accessibility practice withprefers-reduced-motion.Disabling animations for users who prefer reduced motion is an important accessibility consideration. Well done.
packages/tailwind-theme/package.json (1)
1-13: LGTM! Package configuration is well-structured.The package correctly:
- Uses
private: truefor an internal workspace package- Exposes only the intended CSS files via
exports- Lists distributed files in the
filesarrayConsider adding a
"description"field for better discoverability in monorepo tooling.packages/tailwind-theme/README.md (1)
1-30: LGTM! Clear and helpful documentation.The README provides good guidance on:
- Correct import order (fonts → tailwindcss → theme)
- Both local and npm usage patterns
- Key configuration notes
Consider adding a note about the animation utilities (
animate-rise,animate-float) and theirprefers-reduced-motionsupport.src/AudioSharp.App/Styles/app.css (1)
1-3: LGTM! Clean consolidation of styles into the shared theme package.The import order is correct: fonts first, then Tailwind base, then theme overrides. The relative path traversal (
../../../packages/tailwind-theme/) appears correct for the monorepo structure.src/AudioSharp.App/Components/Layout/ReconnectModal.razor.css (1)
23-29: LGTM on CSS variable adoption and no-radius styling.Good use of CSS variables with sensible fallbacks for theming consistency.
src/AudioSharp.App/Components/Layout/MainLayout.razor (1)
3-10: LGTM! Clean layout restructuring.The top-navigation approach with NavMenu and responsive content container is well-structured. The responsive padding classes (
px-5 sm:px-8 lg:px-12) provide good breakpoint scaling.src/AudioSharp.App/Components/Layout/TelemetryPanel.razor (2)
57-66: LGTM! Proper lifecycle management.The component correctly subscribes to the
Changedevent inOnInitializedand unsubscribes inDispose. This prevents memory leaks from lingering event handlers.Also applies to: 130-133
6-37: LGTM on the telemetry card markup.Good use of the theme's CSS custom properties and Tailwind utility classes. The Audio/Text sections are consistently structured with progress bars providing visual feedback.
src/AudioSharp.App/Components/Layout/NavMenu.razor (1)
1-13: LGTM! Clean header-based navigation.The refactor from sidebar with telemetry to a clean sticky header significantly simplifies this component. Good use of responsive classes (
md:flex,md:hidden) for desktop/mobile switching.src/AudioSharp.App/Components/Pages/Home.razor (4)
26-89: LGTM - Clean semantic structure for the aside and TOC.The navigation uses proper anchor links that correspond to section IDs, and the conditional "Recent captures" item correctly depends on
RecentRecords.Count. The vertical line indicator with absolute positioning is a nice visual touch.
91-168: LGTM - Capture section is well-implemented.Button disabled states are properly tied to component state (
IsRecording,IsProcessing), and the status indicator usingdata-stateattributes for CSS-driven styling is a clean pattern. TheInputFilecomponent withaudio/*accept filter is appropriate.
800-805: LGTM - Proper disposal of CancellationTokenSource instances.The
Disposemethod correctly stops the recording timer and cancels/disposes the processing CTS, preventing resource leaks.
170-209: LGTM - Concern display handles optional fields gracefully.The conditional rendering for
Onset,Duration,Impact,Context, andPatientQuoteproperly checks for null/whitespace before rendering, avoiding empty UI elements.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Pull request overview
This pull request refreshes the Tailwind light theme by introducing a shared theme package with standardized design tokens and migrating away from legacy FAST components to native HTML/CSS with Tailwind utilities.
Changes:
- Created a reusable
@audiosharp/tailwind-themepackage with Noto Sans font, amber brand colors, and zero-radius defaults - Refactored all UI components to use Tailwind v4 utility classes instead of FAST Web Components
- Restructured layout with sticky header navigation and sidebar telemetry panel for improved information architecture
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tailwind-theme/* | New shared theme package with fonts, color tokens, and base styles |
| src/AudioSharp.App/Styles/app.css | Updated to import from theme package instead of inline styles |
| src/AudioSharp.App/wwwroot/app.css | Rebuilt minified CSS output with new theme |
| src/AudioSharp.App/Components/Pages/Home.razor | Refactored with Tailwind utilities and improved layout structure |
| src/AudioSharp.App/Components/Layout/TelemetryPanel.razor | Extracted telemetry display into reusable component |
| src/AudioSharp.App/Components/Layout/NavMenu.razor | Simplified to sticky header with native details/summary menu |
| src/AudioSharp.App/Components/Layout/MainLayout.razor | Updated layout structure and error UI styling |
| src/AudioSharp.App/Components/Layout/ReconnectModal.razor.css | Updated to use theme CSS variables |
| src/AudioSharp.App/Components/Layout/*.css | Removed legacy CSS files now handled by Tailwind |
| src/AudioSharp.App/Components/App.razor | Removed FAST components script reference |
Comments suppressed due to low confidence (1)
src/AudioSharp.App/Components/Layout/ReconnectModal.razor.css:105
- The hover and active states for the button are still using the old blue color scheme (#3b6ea2 and #6b9ed2) instead of the new amber brand colors. These should be updated to use brand color variables or appropriate amber shades to maintain consistency with the new theme.
#components-reconnect-modal button:hover {
background-color: #3b6ea2;
}
#components-reconnect-modal button:active {
background-color: #6b9ed2;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .rounded, | ||
| .rounded-sm, | ||
| .rounded-md, | ||
| .rounded-lg, | ||
| .rounded-xl, | ||
| .rounded-2xl, | ||
| .rounded-3xl, | ||
| .rounded-full, | ||
| .rounded-card, | ||
| .file\:rounded-full::file-selector-button { | ||
| border-radius: 0; |
There was a problem hiding this comment.
The markup uses rounded utility classes like rounded-full, rounded-2xl, and rounded-card throughout, but these are all overridden to border-radius: 0 in the theme.css utilities layer (lines 110-121). While this achieves the desired "no-radius" design, it creates a maintainability concern where the class names don't reflect the actual styling. Consider either: 1) removing the rounded classes from the markup since they have no effect, or 2) using a different approach like setting all radius custom properties to 0 without overriding the utility classes, so the intent is clearer.
| <details class="relative md:hidden"> | ||
| <summary class="cursor-pointer text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70">Menu</summary> | ||
| <div class="absolute right-0 mt-3 w-44 rounded-xl border border-white/10 bg-ink-950/95 p-3 text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70 shadow-card"> | ||
| <a class="block py-1 transition hover:text-white" href="#capture">Capture</a> | ||
| <a class="block py-1 transition hover:text-white" href="#concerns">Concerns</a> | ||
| <a class="block py-1 transition hover:text-white" href="#followups">Follow-ups</a> | ||
| <a class="block py-1 transition hover:text-white" href="#bundle">FHIR bundle</a> |
There was a problem hiding this comment.
The mobile navigation menu using <details> and <summary> elements lacks proper ARIA attributes for improved accessibility. Consider adding aria-label="Mobile navigation menu" to the details element and ensuring the summary has clear labeling for screen reader users. Additionally, the dropdown content should have appropriate ARIA attributes to indicate it's a menu.
| <details class="relative md:hidden"> | |
| <summary class="cursor-pointer text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70">Menu</summary> | |
| <div class="absolute right-0 mt-3 w-44 rounded-xl border border-white/10 bg-ink-950/95 p-3 text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70 shadow-card"> | |
| <a class="block py-1 transition hover:text-white" href="#capture">Capture</a> | |
| <a class="block py-1 transition hover:text-white" href="#concerns">Concerns</a> | |
| <a class="block py-1 transition hover:text-white" href="#followups">Follow-ups</a> | |
| <a class="block py-1 transition hover:text-white" href="#bundle">FHIR bundle</a> | |
| <details class="relative md:hidden" aria-label="Mobile navigation menu"> | |
| <summary class="cursor-pointer text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70" aria-label="Open mobile navigation menu">Menu</summary> | |
| <div class="absolute right-0 mt-3 w-44 rounded-xl border border-white/10 bg-ink-950/95 p-3 text-[12px] font-semibold uppercase tracking-[0.3em] text-white/70 shadow-card" role="menu" aria-label="Mobile navigation options"> | |
| <a class="block py-1 transition hover:text-white" href="#capture" role="menuitem">Capture</a> | |
| <a class="block py-1 transition hover:text-white" href="#concerns" role="menuitem">Concerns</a> | |
| <a class="block py-1 transition hover:text-white" href="#followups" role="menuitem">Follow-ups</a> | |
| <a class="block py-1 transition hover:text-white" href="#bundle" role="menuitem">FHIR bundle</a> |
| @theme { | ||
| --font-sans: "Noto Sans", "Segoe UI", system-ui, sans-serif; | ||
| --font-display: "Noto Sans", "Segoe UI", system-ui, sans-serif; | ||
| --font-mono: "Noto Sans", "Segoe UI", system-ui, sans-serif; |
There was a problem hiding this comment.
All three font families (sans, display, and mono) are set to "Noto Sans", which means there's no distinction between monospace and other fonts. This may be intentional for the design, but consider whether code snippets or technical content would benefit from an actual monospace font. The --font-mono variable is typically expected to provide a monospace typeface.
| --font-mono: "Noto Sans", "Segoe UI", system-ui, sans-serif; | |
| --font-mono: "JetBrains Mono", "SFMono-Regular", Menlo, Monaco, Consolas, "Liberation Mono", "Courier New", monospace; |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Addressed the actionable feedback in 2e7c655:\n- added a guard in the recording timer before calling StopRecordingAsync to avoid reentrancy\n- updated reconnect modal hover/active to use brand tokens\n- removed the no-op blur(0px) from the theme\n\nIntentional choices for this PR:\n- keeping --font-mono as Noto Sans to preserve the single-font requirement\n- :root and .dark both map to light tokens (light-only theme for now)\n- zero-radius overrides are intentional to match the provided style\n\nTests: npm run build:css; dotnet test tests/AudioSharp.App.Tests/AudioSharp.App.Tests.csproj |
Summary\n- add shared Tailwind theme package with Noto Sans, amber brand tokens, and no-radius defaults\n- improve light-mode readability with solid surfaces and stronger borders in the main UI and telemetry\n- remove legacy FAST assets and unused layout CSS, rebuild Tailwind output\n\n## Testing\n- npm run build:css\n- dotnet test tests/AudioSharp.App.Tests/AudioSharp.App.Tests.csproj
Summary by CodeRabbit
New Features
Documentation
Style
✏️ Tip: You can customize this high-level summary in your review settings.