Skip to content

Conversation

@9romise
Copy link
Member

@9romise 9romise commented Feb 3, 2026

This PR also unifies the NuxtTime in ‎app/components/BuildEnvironment.vue‎ to the global DateTime component.

@vercel
Copy link

vercel bot commented Feb 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 3, 2026 2:56pm
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 3, 2026 2:56pm
npmx-lunaria Ignored Ignored Feb 3, 2026 2:56pm

Request Review

@9romise 9romise changed the title feat(DateTime): allow toggle relative dates on click feat: allow toggle relative dates on click DateTime Feb 3, 2026
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

The composable useRelativeDates() now returns an object { relativeDates, toggleRelativeDates } instead of a single computed. DateTime.vue's datetime prop accepts string | Date | number, its wrapper changed from span to button and it calls toggleRelativeDates on click, binding aria-pressed to relativeDates. BuildEnvironment.vue replaces NuxtTime with DateTime, dropping locale/relative props. Tests mock the new composable shape and add a click test asserting aria-pressed toggles.

🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description directly relates to the changeset by describing the unification of NuxtTime in BuildEnvironment.vue with the DateTime component.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/nuxt/components/DateTime.spec.ts (1)

8-10: Mock should include toggleRelativeDates to match the new API shape.

🧪 Suggested mock update
 vi.mock('~/composables/useSettings', () => ({
   useRelativeDates: () => ({
     relativeDates: mockRelativeDates,
+    toggleRelativeDates: vi.fn(() => {
+      mockRelativeDates.value = !mockRelativeDates.value
+    }),
   }),
   useSettings: () => ({
     settings: ref({ relativeDates: mockRelativeDates.value }),
   }),
   useAccentColor: () => ({}),
 }))

Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there anywhere the datetime is used within a button or clickable card? I think this might be the case on package search pages.... in this case, embedding a button inside a link will be problematic

@9romise
Copy link
Member Author

9romise commented Feb 3, 2026

We have datetime in the home page (build time), package page (last published, version publish date), compare page (last published), and yes, the package search page.

@9romise
Copy link
Member Author

9romise commented Feb 3, 2026

Perhaps we could add a prop to control whether it renders as a span or a clickable button?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/components/DateTime.vue (2)

12-14: Clarify numeric timestamp units.

The docstring says “timestamp” but doesn’t specify units, which invites seconds-vs-milliseconds confusion. Consider making the unit explicit (or normalising numbers to Date) to avoid ambiguous inputs.

💡 Suggested clarification
-    /** The datetime value (ISO string or Date or timestamp) */
+    /** The datetime value (ISO string, Date, or epoch milliseconds) */

54-85: Consider adding an interactive prop to allow rendering as a <span> when nested in interactive elements.

The DateTime component currently always renders a <button>. Whilst current usage patterns in the codebase (e.g., in Package/Versions.vue) properly separate the component from interactive parents via container <div> elements, adding an optional interactive prop to disable button rendering would make the component more flexible and prevent potential future nesting issues if used in different contexts.

🛠️ Example direction (opt-in interactive mode)
<script setup lang="ts">
 const props = withDefaults(
   defineProps<{
     day?: 'numeric' | '2-digit'
+    /** Render as an interactive button; disable when nested in links/buttons */
+    interactive?: boolean
   }>(),
   {
     day: undefined,
+    interactive: true,
   },
 )
</script>

<template>
-  <button type="button" :aria-pressed="relativeDates" `@click`="toggleRelativeDates">
+  <component
+    :is="interactive ? 'button' : 'span'"
+    v-bind="interactive ? { type: 'button', 'aria-pressed': relativeDates } : {}"
+    v-on="interactive ? { click: toggleRelativeDates } : {}"
+  >
     <ClientOnly>
       <NuxtTime
         v-if="relativeDates"
         :datetime="datetime"
         :title="titleValue"
         relative
         :locale="locale"
       />
       <NuxtTime
         v-else
         :datetime="datetime"
         :title="titleValue"
         :date-style="dateStyle"
         :year="year"
         :month="month"
         :day="day"
         :locale="locale"
       />
       <template `#fallback`>
         <NuxtTime
           :datetime="datetime"
           :title="titleValue"
           :date-style="dateStyle"
           :year="year"
           :month="month"
           :day="day"
           :locale="locale"
         />
       </template>
     </ClientOnly>
-  </button>
+  </component>
</template>

Comment on lines +8 to +10
useRelativeDates: () => ({
relativeDates: mockRelativeDates,
}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add toggleRelativeDates to the mock so clicks actually toggle.

DateTime now calls toggleRelativeDates on click, but the mock only provides relativeDates, so the new interaction test cannot flip state and will fail.

✅ Suggested fix
 vi.mock('~/composables/useSettings', () => ({
   useRelativeDates: () => ({
     relativeDates: mockRelativeDates,
+    toggleRelativeDates: () => {
+      mockRelativeDates.value = !mockRelativeDates.value
+    },
   }),
   useSettings: () => ({
     settings: ref({ relativeDates: mockRelativeDates.value }),
   }),
   useAccentColor: () => ({}),
 }))
📝 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.

Suggested change
useRelativeDates: () => ({
relativeDates: mockRelativeDates,
}),
useRelativeDates: () => ({
relativeDates: mockRelativeDates,
toggleRelativeDates: () => {
mockRelativeDates.value = !mockRelativeDates.value
},
}),

@danielroe
Copy link
Member

I think the problem is that semantically you don't expect <DateTime> to render a button. maybe instead add a new <DateTimeButton> that just wraps it, and then we use that judiciously where needed?

Copy link
Contributor

@wojtekmaj wojtekmaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessibility issue: button isn't labeled in a way that describes what will happen when you activate it. It's a button... With a date. That's all.

Unfortunately I don't immediately see a solution that would both keep the text using DateTime readable and properly label the button.

@9romise 9romise marked this pull request as draft February 4, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants