fix: stabilise modal chart render after transition#1034
fix: stabilise modal chart render after transition#1034graphieros wants to merge 7 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe Modal component declares an emitted 🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/Package/WeeklyDownloadStats.vue (1)
15-21: DoublenextTickmay be overly cautious.The double
await nextTick()pattern is sometimes needed when multiple reactive updates need to flush, but it can be fragile and obscure. Consider whether a singlenextTick()suffices, or document why two are required here.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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)
app/components/Modal.client.vue (1)
45-49:⚠️ Potential issue | 🟠 MajorAddress
closedby="any"attribute browser compatibility gap.The
closedby="any"attribute lacks support in Safari (both macOS and iOS), which remains an open WebKit issue. Chrome and Firefox added support in 2025, but Edge support is shipping in March 2026. For Safari users, the attribute will be silently ignored, resulting in loss of the dismiss-on-outside-click behaviour. Consider implementing a fallback mechanism or feature detection for unsupported browsers, or document this as a known limitation for Safari users.
🧹 Nitpick comments (1)
app/components/Modal.client.vue (1)
56-63: Remove inlinefocus-visibleutility from button.The close button uses an inline
focus-visible:outline-accent/70class, but focus-visible styling for buttons is handled globally inmain.css. Rely on the global rule for consistency.Proposed fix
<button type="button" - class="text-fg-subtle w-5 h-5 hover:text-fg transition-colors duration-200 focus-visible:outline-accent/70 rounded" + class="text-fg-subtle w-5 h-5 hover:text-fg transition-colors duration-200 rounded" :aria-label="$t('common.close')" `@click`="handleModalClose" >Based on learnings: "In the npmx.dev project, focus-visible styling for button and select elements is implemented globally in app/assets/main.css… Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements."
1 participant
Ever since the chart modal was placed inside a dialog element, the chart component started showing flaky behavior:
Fix:
This adds a subtle delay before the content is displayed, but for now it is better than a flaky behavior.