Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Vue page at app/pages/vacations.vue (script setup, TypeScript) providing a localized vacations/maintenance page with stats, decorative icon banner, an interactive fireplace easter egg, and an ICS calendar download. Nuxt routeRules now prerender /vacations and canonical-redirects middleware exempts it. New i18n keys and schema entries plus lunaria locale files (en-GB, en-US) were added. Possibly related PRs
Suggested reviewers
🚥 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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: Daniel Roe <daniel@roe.dev>
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
![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 (4)
i18n/locales/en.json (1)
1112-1158: Consider making the copy less time-relative to avoid future staleness.
meta_description(“reopens in a week”) andwhat.dates(“February 14 – 21”) will read oddly after Feb 2026. If this page might live beyond this specific window, consider including the year and/or using an absolute reopen date (e.g., “Discord reopens February 22, 2026”).app/pages/vacations.vue (3)
111-119: Remove per-buttonfocus-visible:*utilities and rely on the global focus-visible rule.Both buttons add
focus-visible:outline-accent/70, which conflicts with the repo’s “global focus ring” approach forbutton/select.Proposed diff
- class="cursor-pointer inline-flex items-center gap-2 font-mono text-sm text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70 shrink-0" + class="cursor-pointer inline-flex items-center gap-2 font-mono text-sm text-fg-muted hover:text-fg transition-colors duration-200 rounded shrink-0" ... - class="relative shrink-0 cursor-pointer rounded transition-transform duration-200 hover:scale-110 focus-visible:outline-accent/70 w-5 h-5 sm:w-6 sm:h-6" + class="relative shrink-0 cursor-pointer rounded transition-transform duration-200 hover:scale-110 w-5 h-5 sm:w-6 sm:h-6"Based on learnings “focus-visible styling for button and select elements is implemented globally … Do not apply per-element inline utility classes like focus-visible:outline-accent/70 … rely on the global rule.”
Also applies to: 176-190
47-86: Make the.icsdownload more robust (revoke timing + calendar fields).
URL.revokeObjectURL(url)immediately aftera.click()can be flaky in some browsers. Also, addingDTSTAMPimproves compatibility with calendar clients.Proposed diff
const ics = [ 'BEGIN:VCALENDAR', 'VERSION:2.0', 'PRODID:-//npmx//vacations//EN', 'BEGIN:VEVENT', + `DTSTAMP:${fmt(new Date())}`, `DTSTART:${fmt(start)}`, `DTEND:${fmt(end)}`, `SUMMARY:npmx Discord is back!`, `DESCRIPTION:The npmx team is back from vacation. Time to rejoin! https://chat.npmx.dev`, 'STATUS:CONFIRMED', `UID:${uid}`, 'END:VEVENT', 'END:VCALENDAR', ].join('\r\n') const blob = new Blob([ics], { type: 'text/calendar;charset=utf-8' }) const url = URL.createObjectURL(blob) const a = document.createElement('a') a.href = url a.download = 'npmx-discord-reminder.ics' a.click() - URL.revokeObjectURL(url) + // Let the browser start the download before revoking. + setTimeout(() => URL.revokeObjectURL(url), 0) }Verification suggestion: import the generated file into at least Apple Calendar + Google Calendar to confirm it’s accepted as intended.
104-105: Respectprefers-reduced-motionfor the bouncing hero emoji.This animation runs unconditionally; adding a reduced-motion guard keeps the page aligned with accessibility expectations.
Proposed diff
.animate-bounce-slow { animation: bounce 3s infinite; } +@media (prefers-reduced-motion: reduce) { + .animate-bounce-slow { + animation: none; + } +} + `@keyframes` bounce {Also applies to: 298-314
| <button | ||
| type="button" | ||
| class="cursor-pointer inline-flex items-center gap-2 font-mono text-sm text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70 shrink-0" | ||
| @click="router.back()" | ||
| v-if="canGoBack" | ||
| > | ||
| <span class="i-carbon:arrow-left rtl-flip w-4 h-4" aria-hidden="true" /> | ||
| <span class="sr-only sm:not-sr-only">{{ $t('nav.back') }}</span> | ||
| </button> |
There was a problem hiding this comment.
Remove per-button focus-visible utility classes.
These buttons should rely on the global focus-visible styling rather than inline utilities.
🧹 Suggested diff
- class="cursor-pointer inline-flex items-center gap-2 font-mono text-sm text-fg-muted hover:text-fg transition-colors duration-200 rounded focus-visible:outline-accent/70 shrink-0"
+ class="cursor-pointer inline-flex items-center gap-2 font-mono text-sm text-fg-muted hover:text-fg transition-colors duration-200 rounded shrink-0"
...
- class="relative shrink-0 cursor-pointer rounded transition-transform duration-200 hover:scale-110 focus-visible:outline-accent/70 w-5 h-5 sm:w-6 sm:h-6"
+ class="relative shrink-0 cursor-pointer rounded transition-transform duration-200 hover:scale-110 w-5 h-5 sm:w-6 sm:h-6"Based on learnings: In the npmx.dev project, focus-visible styling for button and select elements is implemented globally in app/assets/main.css with the rule: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements in Vue templates or components.
Also applies to: 176-179
| const ics = [ | ||
| 'BEGIN:VCALENDAR', | ||
| 'VERSION:2.0', | ||
| 'PRODID:-//npmx//vacations//EN', | ||
| 'BEGIN:VEVENT', | ||
| `DTSTART:${fmt(start)}`, | ||
| `DTEND:${fmt(end)}`, | ||
| `SUMMARY:npmx Discord is back!`, | ||
| `DESCRIPTION:The npmx team is back from vacation. Time to rejoin! https://chat.npmx.dev`, | ||
| 'STATUS:CONFIRMED', | ||
| `UID:${uid}`, | ||
| 'END:VEVENT', | ||
| 'END:VCALENDAR', | ||
| ].join('\r\n') |
There was a problem hiding this comment.
Consider adding DTSTAMP for ICS spec compliance.
Per RFC 5545, DTSTAMP is a required property for VEVENT. While most calendar apps are lenient, some strict parsers may reject the file without it.
🛠️ Suggested fix
const ics = [
'BEGIN:VCALENDAR',
'VERSION:2.0',
'PRODID:-//npmx//vacations//EN',
'BEGIN:VEVENT',
+ `DTSTAMP:${fmt(new Date())}`,
`DTSTART:${fmt(start)}`,
`DTEND:${fmt(end)}`,
`SUMMARY:npmx Discord is back!`,📝 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.
| const ics = [ | |
| 'BEGIN:VCALENDAR', | |
| 'VERSION:2.0', | |
| 'PRODID:-//npmx//vacations//EN', | |
| 'BEGIN:VEVENT', | |
| `DTSTART:${fmt(start)}`, | |
| `DTEND:${fmt(end)}`, | |
| `SUMMARY:npmx Discord is back!`, | |
| `DESCRIPTION:The npmx team is back from vacation. Time to rejoin! https://chat.npmx.dev`, | |
| 'STATUS:CONFIRMED', | |
| `UID:${uid}`, | |
| 'END:VEVENT', | |
| 'END:VCALENDAR', | |
| ].join('\r\n') | |
| const ics = [ | |
| 'BEGIN:VCALENDAR', | |
| 'VERSION:2.0', | |
| 'PRODID:-//npmx//vacations//EN', | |
| 'BEGIN:VEVENT', | |
| `DTSTAMP:${fmt(new Date())}`, | |
| `DTSTART:${fmt(start)}`, | |
| `DTEND:${fmt(end)}`, | |
| `SUMMARY:npmx Discord is back!`, | |
| `DESCRIPTION:The npmx team is back from vacation. Time to rejoin! https://chat.npmx.dev`, | |
| 'STATUS:CONFIRMED', | |
| `UID:${uid}`, | |
| 'END:VEVENT', | |
| 'END:VCALENDAR', | |
| ].join('\r\n') |
ghostdevv
left a comment
ghostdevv
left a comment
There was a problem hiding this comment.
LGTM, won't merge yet as not sure if I should yet?
3 participants
#1356
closes #1362