-
Notifications
You must be signed in to change notification settings - Fork 127
[DT-3501] reactive timestamp #3108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Creates a new rune that wraps formatDate() to automatically pull format options from the time-format store. This eliminates boilerplate in reactive contexts where components need to pass $timeFormat, $relativeTime, and $timestampFormat to every formatDate() call. Benefits: - Simpler API: just pass the date, optionally override format - Single import instead of multiple store imports - Future-ready: when hourFormat is added, only the rune needs updating - Dollar sign prefix makes reactivity obvious The rune uses $derived.by() to create a reactive computation that automatically tracks changes to timeFormat, relativeTime, and timestampFormat stores.
- Created 12 test cases covering all functionality - Tests verify default behavior, format overrides, timezone handling - Tests confirm reactive store integration - Updated future dates test to match simplified "ago" behavior - All tests passing
- Created isFutureDate() utility in format-time.ts - Encapsulates date/timestamp conversion and comparison logic - Handles ValidTime types (strings, Dates, and Timestamp objects) - Updated timestamp rune to use isFutureDate utility - Rune now automatically chooses "ago" or "from now" based on date - Updated timestamp.svelte component to use the rune - Removed options prop from schedule components (no longer needed) - Updated tests to verify automatic future date detection - All 12 tests passing
Replace formatDate calls with timestamp rune for reactive date formatting in: - workflows-with-new-search.svelte - workflow-summary.svelte - workflow-pending-task.svelte - workflow-details.svelte - timeline-graph.svelte Keeps formatDate for tooltips that intentionally show opposite preference.
Replace formatDate calls with timestamp rune in: - workflow-call-stack.svelte - workflow-callback.svelte - pending-nexus-operation-card.svelte - pending-activity-card.svelte Keep formatDate for special cases with empty relativeLabel.
- Extend timestamp override to accept 'relative' | 'short' | 'medium' | 'long' - 'relative' forces relative time display regardless of user preference - 'short'/'medium'/'long' override format while respecting relativeTime setting - Add tests for new relative override functionality
…-chip to use rune - Add 'absolute' override to force absolute time display - Update workflow-details tooltips to show opposite of user preference using rune - Update dropdown-filter-chip to use 'relative' override - Remove unused formatDate and store imports from both files - Add tests for 'absolute' override - All remaining formatDate calls are intentional (empty relativeLabel cases)
- Remove 'absolute' from TimestampOverride type - Simplify timestamp function back to handling only 'relative' override - Remove 'absolute' override tests - Revert workflow-details.svelte tooltips back to formatDate - Keep dropdown-filter-chip.svelte using 'relative' override
Changed timestamp from a function to a derived store containing a function.
This ensures timestamps automatically update when users change their timezone,
timestamp format, or relative time preferences without requiring page refresh.
Usage:
- Inline: {$timestamp(date)}
- With reactive dates: $derived($timestamp(date))
Updated all components to use $timestamp syntax and fixed tests to use get() helper.
Replace string literal parameter with an options object for better
extensibility and type safety. This allows for more flexible
customization, including the ability to specify a custom relativeLabel.
- Update timestamp rune to accept TimestampOptions object
- Migrate all components from 'short'/'relative' strings to { format: 'short' }
- Replace formatDate calls with $timestamp in pending activity/nexus cards
- Update all tests to use new options object syntax
…orts Create new timezone.ts file to organize timezone-related utilities and resolve circular dependency between format-date.ts and time-format stores. Changes: - Create src/lib/utilities/timezone.ts with timezone types and utilities - Move getLocalTimezone() and getLocalTime() from format-date.ts to timezone.ts to break circular import - Update all component imports to use new timezone.ts module - Update test imports to reflect new file structure This establishes a clean import hierarchy: format-date.ts → timezone.ts (one-way dependency) Previously had circular dependency: format-date.ts ⇄ time-format.ts ⇄ timezone utilities
Move timestamp derived store from the misleading runes/ directory to stores/ where it belongs alongside other stores. Changes: - Move src/lib/runes/timestamp.svelte.ts → src/lib/stores/timestamp.ts - Move src/lib/runes/timestamp.test.ts → src/lib/stores/timestamp.test.ts - Update all imports from '$lib/runes/timestamp' to '$lib/stores/timestamp' - Update test description from 'timestamp rune' to 'timestamp' Rationale: - timestamp is a derived store, not a Svelte 5 rune - Separates configuration stores (time-format.ts) from derived formatting function (timestamp.ts) - Clearer import patterns: components import timestamp for formatting, settings UI imports time-format stores for configuration
Move date comparison utility from format-time.ts to format-date.ts for better cohesion and rename from isFutureDate to isFuture. Changes: - Move isFutureDate function from format-time.ts to format-date.ts - Rename isFutureDate → isFuture for brevity - Export ValidTime type from format-date.ts for external consumers - Use isFuture within formatDate() instead of inline comparison - Update timestamp.ts to import and use isFuture - Remove old isFutureDate from format-time.ts Rationale: - Date comparison belongs with date formatting, not duration formatting - format-date.ts already had inline date comparison logic - Consolidates date utilities in one place - Shorter, clearer name
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| }), | ||
| $timestamp(currentEvent?.eventTime, { format: 'short' }), | ||
| ); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Parameter 'event' implicitly has an 'any' type.
| <Badge>{pendingTask.attempt}</Badge> | ||
| </p> | ||
| <p class="flex items-center gap-4"> | ||
| {translate('workflows.original-scheduled-time')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ 'pendingTask' is possibly 'undefined'.
| <Badge>{$timestamp(pendingTask.originalScheduledTime)}</Badge> | ||
| </p> | ||
| <p class="flex items-center gap-4"> | ||
| {translate('workflows.scheduled-time')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ 'pendingTask' is possibly 'undefined'.
| <Badge>{$timestamp(pendingTask.scheduledTime)}</Badge> | ||
| </p> | ||
| <p class="flex items-center gap-4"> | ||
| {translate('workflows.started-time')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ 'pendingTask' is possibly 'undefined'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this my error, or just seeing it now because I touched the file and danger file is new?
| @@ -0,0 +1,124 @@ | |||
| import { enUS } from 'date-fns/locale'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Could not find a declaration file for module 'date-fns-tz'. '/home/runner/work/ui/ui/node_modules/.pnpm/date-fns-tz@1.3.8_date-fns@2.30.0/node_modules/date-fns-tz/index.js' implicitly has an 'any' type.
|
bd6695e to
e817cf8
Compare
| relative: $relativeTime, | ||
| format: $timestampFormat, | ||
| })} | ||
| content={$timestamp(version.createTime)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens a lot in this PR. A call to formatDate that needs three stores is replaced with a call to $timestamp.
$timestamp is a derived store that derives to a function, so you can call it! The return is reactive to user preference changes.
let formattedTime = $timestamp(someTime)
| format: $timestampFormat, | ||
| }), | ||
| ); | ||
| const eventTime = $derived($timestamp(currentEvent?.eventTime)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we put the call to $timestamp in a $derived, then the return is reactive to user preferences like format and the parameter passed in like currentEvent?.eventTime
| })} | ||
| text={$timestamp(workflow?.executionTime)} | ||
| tooltipText={formatDate(workflow?.executionTime, $timeFormat, { | ||
| relative: !$relativeTime, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shows how a developer can fall back to manually setting up a formatDate if the logic is special, such as this !$relativeTime business.
| <Timestamp | ||
| as="p" | ||
| dateTime={run} | ||
| options={{ relativeLabel: translate('common.from-now') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from-now is handled inside $timestamp
| timeFormatType, | ||
| } from '$lib/stores/time-format'; | ||
| import { getSelectedTimezone } from '$lib/utilities/format-date'; | ||
| import { getTimezone, TIME_UNIT_OPTIONS } from '$lib/utilities/timezone'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were some circular-ish dependencies between stores and utilities. To untangle it, I created the timezone utility.
In general, stores import utilities, so this PR removes some utilities importing from stores (mostly for constants).
| getAdjustedTimeformat, | ||
| TIME_UNIT_OPTIONS, | ||
| Timezones, | ||
| } from '$lib/utilities/timezone'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More unwinding of store/utility interdependency. The timezone file seems to do the trick.
| export const BASE_TIME_FORMAT_OPTIONS = { | ||
| LOCAL: 'local', | ||
| UTC: 'UTC', | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This stuff isn't new. It's pulled into this file from other places like time-format store. Since they're just constants, they make more sense in the utility file.
Description & motivation 💭
This PR adds a new way for a page to render a timestamp such that it always respects users' timezone and format preferences. Rather than the page needing multiple stores to ensure reactivity, that is now captured in the new
timestampfunction.This change is motivated by the task to add 12/24 hour option to the timezone popover. When this choice was added, it wasn't reactive like the short/default/long choice. To make it reactive would require touching each call to formatDate to pass a new reactive parameter.
Instead of doing that, this PR replaces most calls to
formatDatewith the reactive functiontimestamp. Since this function accesses the stores internally, new functionality can be added like the 12/24 hour option, without needing to update each call again. Sincetimestampis actually a derived function, it's return value is reactive to the user's timezone and format preferences.Screenshots (if applicable) 📸
Design Considerations 🎨
Testing 🧪
In progress...
How was this tested 👻
Steps for others to test: 🚶🏽♂️🚶🏽♀️
Checklists
Draft Checklist
Merge Checklist
Issue(s) closed
Docs
Any docs updates needed?