feat: Standard API + Streaming feature#166
Merged
Merged
Conversation
both for lyrics and metadata
feat(docs): New API
Streaming feature
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces an HTMX-aware response helper package ( ChangesHTTP Response Standardization & Route Migration
🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
views/lyrics/queue_header.html (1)
5-13:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit button types to prevent accidental form submits.
At Line 5 and the other view-toggle buttons, missing
typedefaults tosubmit. If this partial is ever embedded inside a form, clicks can trigger unintended submissions.Suggested fix
- <button + <button type="button" hx-get="/lyrics/queue/items" ... - <button + <button type="button" id="view-individual-lyrics" ... - <button + <button type="button" id="view-artist-groups-lyrics" ... - <button + <button type="button" id="view-album-groups-lyrics"Also applies to: 29-40, 42-52, 53-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@views/lyrics/queue_header.html` around lines 5 - 13, The buttons in this partial (e.g., the Refresh Queue button with hx-get="/lyrics/queue/items" and the other view-toggle buttons) lack an explicit type and thus default to type="submit"; update each <button> element (including those with titles like "Refresh Queue" and the other view-toggle buttons referenced) to include type="button" to prevent accidental form submissions when this partial is embedded in a form.Source: Linters/SAST tools
src/features/importing/handlers.go (1)
252-254:⚠️ Potential issue | 🟠 MajorUse
url.PathUnescapeforgroupKeypath params (avoid+-> space).
url.QueryUnescapeapplies query/form decoding rules where+becomes a space, which can corrupt path keys containing literal+. Useurl.PathUnescapeinstead.Suggested fix
- decodedGroupKey, err := url.QueryUnescape(groupKey) + decodedGroupKey, err := url.PathUnescape(groupKey)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/importing/handlers.go` around lines 252 - 254, The code currently decodes the path parameter using url.QueryUnescape (decodedGroupKey, groupKey) which treats '+' as space and can corrupt path keys; replace the call to url.QueryUnescape with url.PathUnescape when decoding groupKey in the handler so literal '+' are preserved, i.e., call url.PathUnescape(groupKey), handle the returned decodedGroupKey and err exactly as before.FEATURE_SPEC_KIT.md (2)
87-93:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unresolved merge conflict markers from the spec.
<<<<<<<,=======, and>>>>>>>were committed into the document and should be resolved before merge.Suggested fix
-<<<<<<< Updated upstream **Error Response Pattern**: - HTMX errors: `src/features/downloading/handlers.go:43-46` - API errors: `src/features/downloading/handlers.go:47-50` - -======= ->>>>>>> Stashed changes🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FEATURE_SPEC_KIT.md` around lines 87 - 93, The file contains unresolved Git merge conflict markers (<<<<<<<, =======, >>>>>>>) around the "Error Response Pattern" block; remove these markers and resolve the conflict by keeping the intended final content for the "Error Response Pattern" section (either the HTMX/API errors lines or the alternative text), ensuring the block reads as a single coherent bullet list and eliminating the "<<<<<<< Updated upstream", "=======" and ">>>>>>> Stashed changes" lines so the document has no conflict artifacts.
99-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate route guidance to match the new direct-path contract.
This still instructs
/ui-group routing, but the migrated routing in this PR uses direct paths. Keeping this text will cause future features to reintroduce stale route patterns.Suggested fix
-- UI routes under `/ui` group for HTMX partials +- UI routes should use direct feature paths (no `/ui` prefix)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@FEATURE_SPEC_KIT.md` around lines 99 - 100, Update the guidance text that references the "/ui" route group to describe the new direct-path contract instead: replace mentions of "UI routes under `/ui` group for HTMX partials" with wording that routes must be registered on direct paths (e.g., "/feature-name/partial") and ensure the sentence about route registration still mandates passing only the feature's own service to handlers; adjust any examples or sample phrasing to show direct-path registration rather than a `/ui` grouped route.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/hosting/respond/respond.go`:
- Around line 12-15: The Section function writes into the provided fiber.Map
without checking for nil, causing a panic if callers pass nil; update Section to
guard against nil by initializing data when nil (e.g., set data = fiber.Map{} or
make(fiber.Map)) before assigning data["Section"] and then proceed to c.Render;
reference the Section function and the data variable when applying this fix.
In `@src/features/importing/handlers.go`:
- Line 90: The toast currently exposes internal error text by passing
err.Error() into respond.ToastErr (e.g., the "Failed to process queue item: %s"
call in handlers.go); change the handler to instead log the full error locally
(using the existing logger or fmt.Printf/log.Printf) and return a stable,
non-sensitive public message to the client via respond.ToastErr (e.g., "Failed
to process queue item") without including err.Error(); apply the same change for
the other occurrence noted (the similar respond.ToastErr call around line 271).
In `@src/features/playlists/handlers.go`:
- Around line 296-300: The MIME declared in respond.Resource for ExportM3U is
"audio/x-mpegurl" but the callback overrides Content-Type to "text/plain",
causing inconsistency; fix by making the callback set Content-Type to
"audio/x-mpegurl" (or remove the callback Content-Type override) so
respond.Resource and the handler callback agree—update the code around
ExportM3U/respond.Resource that sets Content-Type (and keep Content-Disposition
and SendString(m3uContent) intact) so playlistID, filename and m3uContent
behavior is unchanged.
In `@src/features/reorganize/handlers.go`:
- Line 33: The toast currently includes err.Error() which may leak internal
details; instead log the raw error server-side and return a generic message to
the client. Replace the current respond.ToastErr(c,
fiber.StatusInternalServerError, "Failed to start file reorganization job:
"+err.Error()) call with two steps: first log the error (e.g., using the
existing logger or log.Printf/Logger.Errorf with the err value) and then call
respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file
reorganization job") so the client sees a generic message; keep references to
respond.ToastErr, err and fiber.StatusInternalServerError when making the
change.
In `@src/features/streaming/handlers.go`:
- Around line 22-27: The handler currently calls url.QueryUnescape on the "path"
query and lets missing queries fall through to h.service.Stream (causing a 404);
add a presence check for rawPath immediately after rawPath := c.Query("path")
and if rawPath == "" return c.Status(fiber.StatusBadRequest).SendString("missing
path") (or similar message) before attempting url.QueryUnescape and before
calling h.service.Stream so absent query params produce a 400.
In `@views/importing/queue_items_grouped_album.html`:
- Line 141: The grouped album view duplicates the audio player block but lacks
accessible names for icon-only play buttons and sliders; update each play button
(the element with onclick="toggleAudioPlayer(this)" and class "play-btn") to
include an appropriate aria-label (e.g., "Play" / "Pause" dynamically) and
ensure any decorative icons inside are aria-hidden, and update each audio seek
control (the range input/slider in the same player block) to include a
descriptive aria-label or aria-valuemin/aria-valuemax/aria-valuenow attributes
as needed; apply the same changes to the other duplicated instances referenced
(lines with the same play-btn and slider elements) so all icon-only buttons and
sliders in grouped album view have accessible names.
In `@views/importing/queue_items_grouped_artist.html`:
- Line 139: The grouped-artist view currently has icon-only play buttons calling
toggleAudioPlayer(this) and seek controls that lack accessible names; update
each play button (the button elements that call toggleAudioPlayer) to include a
clear accessible name (prefer aria-label="Play" / "Pause" or add visually-hidden
text inside the button and keep decorative SVGs aria-hidden="true"), and ensure
each seek control (the range/input or custom slider element near the player)
includes proper ARIA attributes (aria-label or aria-labelledby, role="slider" if
custom, and aria-valuemin/aria-valuemax/aria-valuenow plus tabindex) so screen
readers can identify and manipulate them; apply the same changes to the other
occurrences noted (lines with similar play buttons and sliders).
In `@views/importing/queue_items.html`:
- Around line 60-61: The new icon-only playback controls (e.g., the play button
with class "play-btn" inside the element id "player-queue-{{.ID}}", the seek
slider elements referenced around the same block and the other similar controls
at the other listed locations) lack accessible names; add appropriate ARIA
attributes and semantics: give each icon-only <button> an aria-label (or
aria-labelledby that references a visible label) describing its action (e.g.,
"Play", "Pause", "Stop"), and for the range/seek inputs provide aria-label or
aria-labelledby plus aria-valuemin/aria-valuemax and aria-valuenow (or
aria-valuetext) updates as the position changes; ensure any custom controls
expose role="slider" if not native and that IDs used by aria-labelledby are
unique per player (use the player-queue-{{.ID}} value to generate unique ids).
In `@views/library/track_overview_panel.html`:
- Line 25: The audio controls in the overview panel (the icon-only play button
that calls toggleAudioPlayer(this) and the range input slider) lack accessible
names; update the button and the input to include descriptive accessible labels
(e.g., add aria-label or aria-labelledby and/or title attributes) so
screen-readers can announce their purpose—ensure the play button
(onclick="toggleAudioPlayer(this)") has an explicit label like "Play audio" /
"Pause audio" (updated dynamically if needed) and the range input has a label
such as "Audio progress" or "Seek audio" (use aria-valuetext or aria-label for
current time where appropriate).
In `@views/partials/scripts.html`:
- Around line 144-157: The code calls audio.play() and immediately sets
icon.className to the pause state, but audio.play() can reject; modify the
click/handler flow around audio.play() so you await or attach a .then/.catch to
audio.play() and only set icon.className = 'fas fa-pause fa-xs' when play
succeeds, and revert or leave as play icon on rejection; ensure you still pause
other audios via the existing querySelectorAll('.audio-player audio') loop and
update their play button icons via closest('.audio-player') and
.querySelector('.play-btn i') as currently implemented, but move the UI toggle
for the clicked audio inside the success path (and handle errors by logging or
setting icon back).
In `@views/playlists/playlist.html`:
- Line 4: The back button element using attributes hx-get="/playlists"
hx-target="`#contenido`" hx-swap="innerHTML" hx-push-url="true" should include an
explicit type attribute to avoid defaulting to submit in form contexts; update
that button by adding type="button" to the element (i.e., ensure the element
with those hx-* attributes has type="button").
In `@views/tag/edit_form.html`:
- Around line 124-131: The play button and seek slider lack accessible names;
update the markup for the elements tied to toggleAudioPlayer and
seekAudioProgress to provide explicit accessible names (e.g., add aria-label or
aria-labelledby on the <button> that calls toggleAudioPlayer and on the <input
type="range"> that calls seekAudioProgress), use descriptive text like "Play
audio"/"Pause audio" for the button (updated dynamically if needed) and "Seek
audio" for the slider, and ensure any referenced label elements or dynamic state
updates are kept in sync with the existing JS handlers so assistive technologies
can identify and control these widgets.
In `@views/tag/search_results_modal.html`:
- Around line 103-107: The button element with
hx-get="/tag/{{$.TrackID}}/select/{{$.ProviderName}}?index={{$index}}" is
missing an explicit type and therefore defaults to submit; add type="button" to
that <button> to prevent accidental form submissions when this modal is rendered
inside a form (update the button element containing the hx-get attribute and the
listed classes).
---
Outside diff comments:
In `@FEATURE_SPEC_KIT.md`:
- Around line 87-93: The file contains unresolved Git merge conflict markers
(<<<<<<<, =======, >>>>>>>) around the "Error Response Pattern" block; remove
these markers and resolve the conflict by keeping the intended final content for
the "Error Response Pattern" section (either the HTMX/API errors lines or the
alternative text), ensuring the block reads as a single coherent bullet list and
eliminating the "<<<<<<< Updated upstream", "=======" and ">>>>>>> Stashed
changes" lines so the document has no conflict artifacts.
- Around line 99-100: Update the guidance text that references the "/ui" route
group to describe the new direct-path contract instead: replace mentions of "UI
routes under `/ui` group for HTMX partials" with wording that routes must be
registered on direct paths (e.g., "/feature-name/partial") and ensure the
sentence about route registration still mandates passing only the feature's own
service to handlers; adjust any examples or sample phrasing to show direct-path
registration rather than a `/ui` grouped route.
In `@src/features/importing/handlers.go`:
- Around line 252-254: The code currently decodes the path parameter using
url.QueryUnescape (decodedGroupKey, groupKey) which treats '+' as space and can
corrupt path keys; replace the call to url.QueryUnescape with url.PathUnescape
when decoding groupKey in the handler so literal '+' are preserved, i.e., call
url.PathUnescape(groupKey), handle the returned decodedGroupKey and err exactly
as before.
In `@views/lyrics/queue_header.html`:
- Around line 5-13: The buttons in this partial (e.g., the Refresh Queue button
with hx-get="/lyrics/queue/items" and the other view-toggle buttons) lack an
explicit type and thus default to type="submit"; update each <button> element
(including those with titles like "Refresh Queue" and the other view-toggle
buttons referenced) to include type="button" to prevent accidental form
submissions when this partial is embedded in a form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3501b96a-a25e-417b-bbaa-d672821f3f17
📒 Files selected for processing (67)
FEATURE_SPEC_KIT.mddocs/api.mdsrc/features/config/handlers.gosrc/features/config/routes.gosrc/features/downloading/handlers.gosrc/features/downloading/routes.gosrc/features/hosting/respond/respond.gosrc/features/hosting/server.gosrc/features/importing/handlers.gosrc/features/importing/routes.gosrc/features/importing/service.gosrc/features/jobs/handlers.gosrc/features/jobs/routes.gosrc/features/library/handlers.gosrc/features/library/routes.gosrc/features/library/service.gosrc/features/lyrics/handlers.gosrc/features/lyrics/lyrics_job.gosrc/features/lyrics/routes.gosrc/features/metadata/handlers.gosrc/features/metadata/routes.gosrc/features/metrics/handlers.gosrc/features/metrics/routes.gosrc/features/playlists/handlers.gosrc/features/playlists/routes.gosrc/features/reorganize/handlers.gosrc/features/reorganize/routes.gosrc/features/streaming/handlers.gosrc/features/streaming/routes.gosrc/features/streaming/service.gosrc/features/ui/handlers.gosrc/features/ui/routes.gosrc/main.goviews/cards/quick_actions.htmlviews/config/config_form.htmlviews/importing/queue_header.htmlviews/importing/queue_items.htmlviews/importing/queue_items_grouped_album.htmlviews/importing/queue_items_grouped_artist.htmlviews/library/track_overview_panel.htmlviews/library/unified_search_list.htmlviews/lyrics/queue_header.htmlviews/lyrics/queue_items.htmlviews/metrics/overview.htmlviews/partials/job_status.htmlviews/partials/main.htmlviews/partials/navbar.htmlviews/partials/scripts.htmlviews/partials/sidebar.htmlviews/playlists/add_to_playlist_modal.htmlviews/playlists/playlist.htmlviews/sections/analyze.htmlviews/sections/analyze_files.htmlviews/sections/analyze_lyrics.htmlviews/sections/analyze_metadata.htmlviews/sections/dashboard.htmlviews/sections/download.htmlviews/sections/import.htmlviews/sections/jobs.htmlviews/sections/library.htmlviews/sections/playlists.htmlviews/sections/settings.htmlviews/sections/tag.htmlviews/tag/edit_form.htmlviews/tag/lyrics_buttons.htmlviews/tag/metadata_buttons.htmlviews/tag/search_results_modal.html
Comment on lines
+12
to
+15
| func Section(c *fiber.Ctx, section string, data fiber.Map) error { | ||
| if c.Get("HX-Request") != "true" { | ||
| data["Section"] = section | ||
| return c.Render("main", data) |
There was a problem hiding this comment.
Guard Section against nil map writes.
Line 14 writes data["Section"] unconditionally. If a caller passes nil for data, this panics (assignment to entry in nil map) and turns a request into a 500.
Suggested fix
func Section(c *fiber.Ctx, section string, data fiber.Map) error {
+ if data == nil {
+ data = fiber.Map{}
+ }
if c.Get("HX-Request") != "true" {
data["Section"] = section
return c.Render("main", data)
}
return c.Render("sections/"+section, data)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func Section(c *fiber.Ctx, section string, data fiber.Map) error { | |
| if c.Get("HX-Request") != "true" { | |
| data["Section"] = section | |
| return c.Render("main", data) | |
| func Section(c *fiber.Ctx, section string, data fiber.Map) error { | |
| if data == nil { | |
| data = fiber.Map{} | |
| } | |
| if c.Get("HX-Request") != "true" { | |
| data["Section"] = section | |
| return c.Render("main", data) | |
| } | |
| return c.Render("sections/"+section, data) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/hosting/respond/respond.go` around lines 12 - 15, The Section
function writes into the provided fiber.Map without checking for nil, causing a
panic if callers pass nil; update Section to guard against nil by initializing
data when nil (e.g., set data = fiber.Map{} or make(fiber.Map)) before assigning
data["Section"] and then proceed to c.Render; reference the Section function and
the data variable when applying this fix.
Comment on lines
+296
to
+300
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | ||
| c.Set("Content-Type", "text/plain") | ||
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | ||
| return c.SendString(m3uContent) | ||
| }) |
There was a problem hiding this comment.
MIME type mismatch in ExportM3U.
The respond.Resource call specifies "audio/x-mpegurl" as the MIME type, but the callback then overrides Content-Type to "text/plain". This inconsistency may cause unexpected behavior if respond.Resource uses the MIME type for caching, content negotiation, or ETag generation.
Consider aligning these values—either use "audio/x-mpegurl" consistently (the standard MIME for M3U files) or remove the duplicate Content-Type setting in the callback.
Suggested fix
- return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error {
- c.Set("Content-Type", "text/plain")
+ return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error {
+ c.Set("Content-Type", "audio/x-mpegurl")
c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename))
return c.SendString(m3uContent)
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | |
| c.Set("Content-Type", "text/plain") | |
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | |
| return c.SendString(m3uContent) | |
| }) | |
| return respond.Resource(c, "audio/x-mpegurl", fmt.Sprintf("%s/playlists/%s/export", c.BaseURL(), playlistID), func() error { | |
| c.Set("Content-Type", "audio/x-mpegurl") | |
| c.Set("Content-Disposition", fmt.Sprintf("inline; filename=\"%s\"", filename)) | |
| return c.SendString(m3uContent) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/playlists/handlers.go` around lines 296 - 300, The MIME declared
in respond.Resource for ExportM3U is "audio/x-mpegurl" but the callback
overrides Content-Type to "text/plain", causing inconsistency; fix by making the
callback set Content-Type to "audio/x-mpegurl" (or remove the callback
Content-Type override) so respond.Resource and the handler callback agree—update
the code around ExportM3U/respond.Resource that sets Content-Type (and keep
Content-Disposition and SendString(m3uContent) intact) so playlistID, filename
and m3uContent behavior is unchanged.
| return c.Status(fiber.StatusInternalServerError).Render("toast/toastError", fiber.Map{ | ||
| "Msg": "Failed to start file reorganization job: " + err.Error(), | ||
| }) | ||
| return respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file reorganization job: "+err.Error()) |
There was a problem hiding this comment.
Do not include err.Error() in client-facing toast text.
This can leak internal service details to users. Log the raw error server-side, but return a generic message to the client.
Suggested fix
- return respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file reorganization job: "+err.Error())
+ return respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file reorganization job")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file reorganization job: "+err.Error()) | |
| return respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file reorganization job") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/reorganize/handlers.go` at line 33, The toast currently includes
err.Error() which may leak internal details; instead log the raw error
server-side and return a generic message to the client. Replace the current
respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to start file
reorganization job: "+err.Error()) call with two steps: first log the error
(e.g., using the existing logger or log.Printf/Logger.Errorf with the err value)
and then call respond.ToastErr(c, fiber.StatusInternalServerError, "Failed to
start file reorganization job") so the client sees a generic message; keep
references to respond.ToastErr, err and fiber.StatusInternalServerError when
making the change.
|
|
||
| <!-- Audio Player --> | ||
| <div class="audio-player px-4 py-3 border-b border-gray-200 dark:border-gray-800 flex items-center gap-2 shrink-0"> | ||
| <button type="button" onclick="toggleAudioPlayer(this)" |
There was a problem hiding this comment.
Add accessible labels to overview panel audio controls.
The icon-only play button and range input need accessible names for screen-reader usability.
Also applies to: 29-31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@views/library/track_overview_panel.html` at line 25, The audio controls in
the overview panel (the icon-only play button that calls toggleAudioPlayer(this)
and the range input slider) lack accessible names; update the button and the
input to include descriptive accessible labels (e.g., add aria-label or
aria-labelledby and/or title attributes) so screen-readers can announce their
purpose—ensure the play button (onclick="toggleAudioPlayer(this)") has an
explicit label like "Play audio" / "Pause audio" (updated dynamically if needed)
and the range input has a label such as "Audio progress" or "Seek audio" (use
aria-valuetext or aria-label for current time where appropriate).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactoring
Documentation