Replace mixed emoji/Font Awesome/Open Iconic with unified SVG icon system - #134
Conversation
…stem - Add AppIcon shared SVG component (38 Tabler-style icons) replacing emoji and icon font usage across the dashboard and nav - Update PlaybackCard, MediaTabs, MediaLists, QueuePanel, IndexPage, NavMenu, MainLayout to use AppIcon - Remove Font Awesome CDN from _Host.cshtml (no fa-* refs remain) - Fix Calendar.razor keyboard event handler (invalid expression) - Add icon-spin CSS animation for loading states in QueuePanel - Add MarkupString support in SectionCard for SVG icon strings
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| @@ -142,12 +146,15 @@ | |||
|
|
|||
| <section class="card media-section"> | |||
There was a problem hiding this comment.
CRITICAL: Copy-paste error - YouTube Music section incorrectly uses Spotify icon. Should be AppIconType.Video or AppIconType.Youtube.
| @@ -142,12 +146,15 @@ | |||
|
|
|||
| <section class="card media-section"> | |||
| <div class="card-header d-flex align-items-center justify-content-between gap-3"> | |||
There was a problem hiding this comment.
CRITICAL: Copy-paste error - Section header shows "Spotify Tracks" but this is the YouTube Music section. Should say "YouTube Music".
| <h5 class="mb-0">▶️ YouTube Music</h5> | ||
| <div class="d-flex align-items-center gap-2"> | ||
| <AppIcon Icon="AppIcon.AppIconType.Spotify" Width="18" Height="18" /> | ||
| <h5 class="mb-0">Spotify Tracks</h5> |
There was a problem hiding this comment.
CRITICAL: Copy-paste error - YouTube Music section incorrectly calls ToggleSpotifyEditMode instead of ToggleYouTubeEditMode.
| <div class="d-flex align-items-center gap-2"> | ||
| <AppIcon Icon="AppIcon.AppIconType.Spotify" Width="18" Height="18" /> | ||
| <h5 class="mb-0">Spotify Tracks</h5> | ||
| </div> |
There was a problem hiding this comment.
CRITICAL: Copy-paste error - aria-label says "Toggle Spotify playlists" but this is the YouTube Music section.
| aria-label="Toggle YouTube edit mode"> | ||
| <i class="fa @(IsYouTubeEditMode ? "fa-check" : "fa-pencil")" aria-hidden="true"></i> | ||
| <span class="visually-hidden">@(IsYouTubeEditMode ? "Exit edit mode" : "Edit YouTube links")</span> | ||
| @onclick="ToggleSpotifyEditMode" |
There was a problem hiding this comment.
CRITICAL: Copy-paste error - Uses IsSpotifyEditMode but this section body uses IsYouTubeEditMode (line 161), causing the edit toggle to not work correctly.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge OverviewAll previously identified copy-paste errors have been fixed in this diff:
Changes Reviewed
Files Reviewed (9 files)
Summary: This PR successfully replaces the mixed emoji/Font Awesome/Open Iconic icon stack with a unified self-hosted SVG icon component. The copy-paste errors from previous reviews have been addressed. The code changes are clean and follow proper accessibility practices. |
There was a problem hiding this comment.
CRITICAL: Copy-paste error - The "Play Spotify URL" section header (lines 216-224) was incorrectly replaced with YouTube Music content. The body (lines 226-237) still contains Spotify URL input (placeholder="Enter Spotify URL", aria-label="Spotify URL", PlaySpotifyUrl callback). This section should show "Play Spotify URL" with a Spotify icon, not "YouTube Music" with a Video icon and YouTube edit toggle.
<section class="card media-section">
<div class="card-header d-flex align-items-center gap-2">
<AppIcon Icon="AppIcon.AppIconType.Spotify" Width="18" Height="18" />
<h5 class="mb-0">Play Spotify URL</h5>
</div>
| <h5 class="mb-0">🎧 Play Spotify URL</h5> | ||
| <div class="card-header d-flex align-items-center justify-content-between gap-3"> | ||
| <div class="d-flex align-items-center gap-2"> | ||
| <AppIcon Icon="AppIcon.AppIconType.Video" Width="18" Height="18" /> |
There was a problem hiding this comment.
CRITICAL: Copy-paste error - Uses Video icon (YouTube) but this is the "Play Spotify URL" section. Should use Spotify icon instead.
db3ad64 to
443fbaf
Compare
|
/oc summarize |
PR Summary: Unified SVG Icon SystemPR # (broadcast-studio-redesign-v2 → nightly) | Author: Darkatek7 | Files: 16 (+330/-119) What this PR does
Components updatedPlaybackCard, MediaTabs, MediaLists, QueuePanel, IndexPage, NavMenu, MainLayout, SectionCard Outstanding Issue (CRITICAL)
This was flagged in the latest review and needs fixing before merge. |
|
/oc review |
Co-authored-by: Darkatek7 <Darkatek7@users.noreply.github.com>
|
Build passes. Fixed the copy-paste error in the "Play Spotify URL" section header:
Files reviewed: 16 files in PR |
…d, fa icons replaced
1 participant
Summary
Replace the mixed emoji/Font Awesome/Open Iconic icon stack with a single unified SVG icon component (
AppIcon.razor) across the dashboard and navigation._Host.cshtml"Enter"inside double-quoted attribute) — fixed with a proper handler methodicon-spinCSS keyframe for loading statesScope
Dashboard components and nav — exactly what the v1 redesign touched. Other pages (ConfigPage, StationLookup, UserManagement, Logs, etc.) still have emoji and can be updated separately.
Validation
docker build .— passes, 0 errorsnightly, targetingnightly