Update batch #2 - #19
Conversation
…s and improve synchronization
…rate graph hover functionality
There was a problem hiding this comment.
Pull request overview
This PR addresses multiple UI and UX improvements including fixing the favicon configuration, adding URL fragment navigation for tabs, improving the timeline dot positioning, updating rank badge styling, and enhancing the SVG graph hover interaction accuracy.
- Fixed favicon configuration to use favicon.ico instead of icon.png
- Added URL fragment support for tab navigation with hash change listeners and two-way sync
- Enhanced graph hover coordinate mapping using SVG transformation matrices for better accuracy
- Updated rank badge styling with refined visual emphasis and improved timeline dot positioning
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| nuxt.config.ts | Updates favicon link references from icon.png to favicon.ico with corresponding MIME type change |
| app/app.vue | Adds URL fragment navigation, improves SVG graph coordinate mapping, refines rank badge styling, repositions timeline dots, and adds keyboard interaction support for deep stats cards |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| link: [ | ||
| { rel: 'icon', type: 'image/png', href: '/icon.png' }, | ||
| { rel: 'apple-touch-icon', href: '/icon.png' } | ||
| { rel: 'icon', type: 'image/x-icon', href: '/favicon.ico' }, |
There was a problem hiding this comment.
The MIME type 'image/x-icon' is outdated. Modern browsers and standards recommend using 'image/png' for PNG favicons or leaving the type attribute unspecified for .ico files, as browsers will automatically detect the correct type. The .ico extension suggests this is an ICO file, so the type attribute should either be removed or changed to the non-standard but commonly used 'image/vnd.microsoft.icon'.
| { rel: 'icon', type: 'image/x-icon', href: '/favicon.ico' }, | |
| { rel: 'icon', href: '/favicon.ico' }, |
| try { | ||
| history.replaceState(null, '', `#${frag}`) | ||
| } catch (e) { | ||
| if (typeof window !== 'undefined') window.location.hash = `#${frag}` |
There was a problem hiding this comment.
In the catch block fallback, setting window.location.hash directly will trigger the hashchange event listener, which could cause the setTabFromHash function to run again. While this likely won't cause an infinite loop due to the guard condition in setTabFromHash, it creates an unnecessary re-execution. Consider adding a flag to temporarily disable the hashchange listener during programmatic updates, or simply remove the fallback since history.replaceState has broad browser support.
| if (typeof window !== 'undefined') window.location.hash = `#${frag}` | |
| // Swallow errors from history.replaceState; avoid hash fallback to | |
| // prevent triggering the hashchange listener unnecessarily. |
|
|
||
| <div class="flex flex-row flex-wrap gap-2"> | ||
| <div | ||
| <div |
There was a problem hiding this comment.
Inconsistent indentation: this line has extra indentation (14 spaces) compared to the surrounding code. The opening div tag should align with the other elements in the flex container.
| badge.tone === 'badge-a' ? 'border border-white/50 text-white' : '', | ||
| badge.tone === 'badge-s' ? '' : '', | ||
| badge.tone === 'badge-ss' ? '' : '', | ||
| :class="[ |
There was a problem hiding this comment.
Inconsistent indentation: this line has extra indentation (14 spaces) compared to the parent element. The :class attribute should align with the class attribute above it.
Summary
Why
Why is this change needed?
Testing
npm run buildSecurity / privacy
Notes
Im also considering adding ability to copy a replay link