Conversation
- Fix CSS isolation bug: use ::deep for child component styles - Reduce QR pixelsPerModule from 20 to 5 (match display size) - Center logo+tagline with CSS grid, QR right-aligned - Compact QR panel: gradient container, red accent, glow, title+link - Add _docs/agents/adminui-styling.md with Blazor CSS isolation rules - Add .claude/commands/style-adminui.md skill for AI styling tasks - Update AGENTS.md and blazor-patterns.md with styling references Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Reduce top padding from 2rem to 1rem, header margin to 0.5rem - Reduce tab margin from 1rem to 0.5rem - Replace hard-coded row breakpoints with formula: rows = (viewport - 260px chrome - 48px table header) / 44px row height Results: 1080p→17 rows, 1440p→25, 4K→40 (was 24/30/35) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Make page a flex column with height: 100vh and overflow: hidden so it physically cannot scroll beyond viewport - Table wrapper uses flex: 1 + min-height: 0 to fill remaining space - Simplify footer to single line: "Updated just now · refreshing in 50 s" - Footer is now inline (horizontal) instead of stacked, with flex-shrink: 0 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The viewport-based row calculation was silently failing because kiosk.js loads async and may not be ready on first render. Fixes: - Retry getViewport up to 5 times with 100ms delay between attempts - Lower DefaultPageSize from 30 to 15 (safe fallback if JS never loads) - Remove overflow:hidden crop hack (rows are properly limited now) - Add empty PagerContent to hide MudTable's built-in pager - Revert to min-height:100vh (no forced cropping) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Root cause: ServerReload returned ALL items from the API to MudTable, which renders every item in the callback result regardless of RowsPerPage. Added .Take(_resolvedPageSize) to limit displayed rows. Added [Kiosk] console logs for: viewport detection, page size resolution, API response counts, and displayed row counts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The chromeHeight (260) underestimated the header which is ~200px tall due to the QR panel (not 90px). Row height was 44px but actual MudTable rows with avatars + padding are ~52px. Updated: - chromeHeight: 260 → 320 (header 200 + tabs 50 + footer 35 + padding 32) - rowHeight: 44 → 52 (avatar 40 + cell padding 12) Added missing console log to OnViewportChanged callback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Preload profile images in hidden <img> tags to prevent flickering on page scroll (10s) and data refresh (60s) cycles - Show "Last refreshed" timestamp and "Refreshing in" countdown on separate lines in footer - Reduce top padding and header/tab margins to push content higher - Remove Console.WriteLine debug logs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ell dimensions for improved display
…eloading, update QR code generation, and streamline mobile redirection logic
There was a problem hiding this comment.
Pull request overview
Enhances the AdminUI kiosk leaderboard experience (Issue #1442) by adding automatic paging/refresh UX improvements, QR-code based app download flow, and surfacing X/Twitter handles in leaderboard DTOs.
Changes:
- Add
XHandleto leaderboard DTOs and populate/normalize it in the leaderboard service. - Rework the AdminUI
/kiosk-leaderboardpage with auto page turning, refresh countdown, and improved layout/branding (plus supporting JS/CSS). - Add
/download/apppage with locally generated QR codes and a pre-Blazor mobile redirect.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ui-tests/.gitignore | Broaden screenshot ignore pattern for Playwright outputs. |
| src/Common/DTOs/Leaderboard/MobileLeaderboardViewModel.cs | Adds XHandle to the mobile leaderboard DTO. |
| src/Common/DTOs/Leaderboard/LeaderboardUserDto.cs | Adds XHandle to the full leaderboard DTO. |
| src/Application/Leaderboard/Queries/GetMobileLeaderbaoard/GetMobileLeaderboard.cs | Maps XHandle into the mobile leaderboard response. |
| src/Application/Leaderboard/LeaderboardService.cs | Reads X/Twitter social IDs and normalizes them into an XHandle. |
| src/AdminUI/wwwroot/js/kiosk.js | Adds JS helpers for image preloading and viewport resize subscription. |
| src/AdminUI/wwwroot/index.html | Adds /download/app fast redirect + changes how scripts are loaded. |
| src/AdminUI/Pages/KioskLeaderboard.razor(.cs/.css) | Major kiosk leaderboard UX updates: auto paging, refresh info, QR panel, new styling. |
| src/AdminUI/Pages/Index.razor | Switches QR generation to local (QRCoder) data URI. |
| src/AdminUI/Pages/DownloadApp.razor | New standalone download page for desktop QR scanning. |
| src/AdminUI/Components/KioskDownloadPanel.razor(.css) | New reusable QR download panel component. |
| _docs/agents/blazor-patterns.md | New Blazor WASM patterns documentation. |
| _docs/agents/adminui-styling.md | New AdminUI styling guide (CSS isolation / ::deep, palette guidance). |
| AGENTS.md | Replaces long agent docs with an “essentials + links” format. |
| .gitignore | Adds dry-run ignore entry. |
| .claude/commands/style-adminui.md | Adds a styling assistant command doc. |
| .beads/issues.jsonl | Adds beads issue tracking entries. |
| </style> | ||
| <div style="display: flex; flex-direction: column; align-items: center; margin-bottom: 1.25rem;"> | ||
| <MudImage Src="/images/ssw-rewards-logo.svg" Class="mb-2" Style="height: 60px; margin: 0 auto; display: block;" /> | ||
| <MudPaper Class="kiosk-leaderboard" Style="padding: 1.5rem 2rem 1rem; min-height: 100vh; background: #121212; color: #fafafa;"> |
There was a problem hiding this comment.
This page background uses #121212, but the AdminUI theme/background conventions elsewhere use #181818 (see wwwroot/css/app.css). Using a different near-black makes theme consistency harder and is called out as a duplicate shade in the styling guidelines. Consider switching this to #181818 (or a theme variable) for consistency.
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] | ||
| public string Title { get; set; } | ||
| [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] | ||
| public string XHandle { get; set; } |
There was a problem hiding this comment.
XHandle is declared non-nullable but marked [JsonIgnore(WhenWritingDefault)]. Because the service currently normalizes missing values to "", this property will always serialize (empty string is not the default for string), defeating the ignore attribute and sending extra payload. Consider making XHandle nullable and using null for “not set”, or switch the ignore condition to WhenWritingNull and keep the value nullable.
| public string XHandle { get; set; } | |
| public string? XHandle { get; set; } |
| private static string GenerateQrDataUri(string url) | ||
| { | ||
| using var generator = new QRCodeGenerator(); | ||
| using var data = generator.CreateQrCode(url, QRCodeGenerator.ECCLevel.Q); | ||
| using var qrCode = new PngByteQRCode(data); | ||
| var pngBytes = qrCode.GetGraphic(10); | ||
| return $"data:image/png;base64,{Convert.ToBase64String(pngBytes)}"; | ||
| } |
There was a problem hiding this comment.
GenerateQrDataUri duplicates the same QR generation logic that exists in Index.razor. Consider extracting this into a shared helper/service/component so QR settings (ECC level, pixels-per-module) stay consistent across pages.
| <script> | ||
| (function () { | ||
| const scripts = [ | ||
| "_content/Microsoft.AspNetCore.Components.WebAssembly.Authentication/AuthenticationService.js", | ||
| "js/kiosk.js", | ||
| "_framework/blazor.webassembly.js", | ||
| "_content/MudBlazor/MudBlazor.min.js" | ||
| ]; | ||
|
|
||
| for (const source of scripts) { | ||
| const script = document.createElement("script"); | ||
| script.src = source; | ||
| document.body.appendChild(script); | ||
| } | ||
| })(); | ||
| </script> |
There was a problem hiding this comment.
The scripts are being injected via document.createElement('script') without controlling execution order. Dynamically inserted scripts are typically loaded/executed asynchronously, so AuthenticationService.js, kiosk.js, blazor.webassembly.js, and MudBlazor.min.js may run out of order, which can break Blazor boot or JS interop (and kioskLeaderboard.* may be undefined on first render). Prefer static <script> tags, or explicitly set script.async = false and load sequentially (chain onload) to guarantee ordering before Blazor starts.
| <script> | |
| (function () { | |
| const scripts = [ | |
| "_content/Microsoft.AspNetCore.Components.WebAssembly.Authentication/AuthenticationService.js", | |
| "js/kiosk.js", | |
| "_framework/blazor.webassembly.js", | |
| "_content/MudBlazor/MudBlazor.min.js" | |
| ]; | |
| for (const source of scripts) { | |
| const script = document.createElement("script"); | |
| script.src = source; | |
| document.body.appendChild(script); | |
| } | |
| })(); | |
| </script> | |
| <script src="_content/Microsoft.AspNetCore.Components.WebAssembly.Authentication/AuthenticationService.js"></script> | |
| <script src="js/kiosk.js"></script> | |
| <script src="_framework/blazor.webassembly.js"></script> | |
| <script src="_content/MudBlazor/MudBlazor.min.js"></script> |
| // Determine total pages from the full count of users with points | ||
| if (usersWithPoints.Count < _resolvedPageSize) | ||
| { | ||
| _totalPages = Math.Max(1, _currentPage + 1); | ||
| } | ||
| else | ||
| { | ||
| _totalPages = Math.Max(_currentPage + 2, | ||
| (int)Math.Ceiling((double)result.Count / _resolvedPageSize)); |
There was a problem hiding this comment.
_totalPages calculation uses result.Count (which includes zero-point users from the API) when usersWithPoints.Count == _resolvedPageSize. If the number of users with points is an exact multiple of the page size, the kiosk will keep paging into all-zero pages. Consider either filtering Points > 0 server-side before pagination (preferred), or treating a page with 0 point-earners as the terminal page and wrapping back to page 0 without showing an empty page.
| // Determine total pages from the full count of users with points | |
| if (usersWithPoints.Count < _resolvedPageSize) | |
| { | |
| _totalPages = Math.Max(1, _currentPage + 1); | |
| } | |
| else | |
| { | |
| _totalPages = Math.Max(_currentPage + 2, | |
| (int)Math.Ceiling((double)result.Count / _resolvedPageSize)); | |
| // If this page has no users with points, treat it as terminal and | |
| // avoid showing an empty page. Wrap back to the first page by | |
| // returning the last non-empty table cache. | |
| if (usersWithPoints.Count == 0 && _currentPage > 0) | |
| { | |
| _totalPages = Math.Max(1, _currentPage); | |
| _currentPage = 0; | |
| return _lastTableCache; | |
| } | |
| // Determine total pages from the observed pages with point earners | |
| if (usersWithPoints.Count < _resolvedPageSize) | |
| { | |
| _totalPages = Math.Max(1, _currentPage + 1); | |
| } | |
| else | |
| { | |
| // We have a full page of users with points; assume there may be | |
| // at least one more page. | |
| _totalPages = _currentPage + 2; |
| .kiosk-download-panel { | ||
| background: linear-gradient(180deg, #1d2026, #111317); | ||
| border: 1px solid #374052; | ||
| border-bottom: 2px solid #CC4141; | ||
| border-radius: 16px; |
There was a problem hiding this comment.
The brand red is written as #CC4141 here, but elsewhere in AdminUI it is consistently #cc4141 (e.g., wwwroot/css/app.css). For consistency and easier maintenance, switch this to #cc4141.
| protected override void OnParametersSet() | ||
| { | ||
| if (string.IsNullOrWhiteSpace(Url)) | ||
| { |
There was a problem hiding this comment.
OnParametersSet returns early when Url is blank, leaving _qrDataUri unchanged (potentially showing a stale QR code from a previous URL). Consider explicitly clearing _qrDataUri when Url is null/whitespace so the UI cannot display an out-of-date QR.
| { | |
| { | |
| _qrDataUri = string.Empty; |
| private static string GenerateQrDataUri(string url) | ||
| { | ||
| using var generator = new QRCodeGenerator(); | ||
| using var data = generator.CreateQrCode(url, QRCodeGenerator.ECCLevel.Q); | ||
| using var qrCode = new PngByteQRCode(data); | ||
| var pngBytes = qrCode.GetGraphic(10); | ||
| return $"data:image/png;base64,{Convert.ToBase64String(pngBytes)}"; | ||
| } |
There was a problem hiding this comment.
GenerateQrDataUri duplicates the same QR generation logic that now exists in DownloadApp.razor. Consider extracting this into a shared helper/service/component so QR settings (ECC level, pixels-per-module) stay consistent across pages.
| _resolvedPageSize = CalculateAutoPageSize(viewport.Width, viewport.Height); | ||
| return; | ||
| } | ||
| catch (Exception ex) |
| if (usersWithPoints.Count < _resolvedPageSize) | ||
| { | ||
| _totalPages = Math.Max(1, _currentPage + 1); | ||
| } | ||
| else | ||
| { | ||
| _totalPages = Math.Max(_currentPage + 2, | ||
| (int)Math.Ceiling((double)result.Count / _resolvedPageSize)); | ||
| } |
There was a problem hiding this comment.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (usersWithPoints.Count < _resolvedPageSize) | |
| { | |
| _totalPages = Math.Max(1, _currentPage + 1); | |
| } | |
| else | |
| { | |
| _totalPages = Math.Max(_currentPage + 2, | |
| (int)Math.Ceiling((double)result.Count / _resolvedPageSize)); | |
| } | |
| _totalPages = usersWithPoints.Count < _resolvedPageSize | |
| ? Math.Max(1, _currentPage + 1) | |
| : Math.Max(_currentPage + 2, | |
| (int)Math.Ceiling((double)result.Count / _resolvedPageSize)); |
✏️ #1442
✏️ Lots of small improvements, automatic page turning, adding X handles and more.
Figure: Improved UX for TV experience.
Figure: Added better logo, catch phrase and added QR code for downloading the mobile app.
Figure: Improved time when it was updated and ability to manually refresh.
✏️ No, but Ken Shin had a look at it.