-
Notifications
You must be signed in to change notification settings - Fork 93
feat(functions): Provide formatDateTime
function
#6543
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: stable8
Are you sure you want to change the base?
Conversation
Signed-off-by: Ferdinand Thiessen <[email protected]>
Basically a non-reactive way of `useFormatDateTime` Signed-off-by: Ferdinand Thiessen <[email protected]>
/backport to next |
Interface seems confusing to me.
What about:
/**
* Current
*/
// Complex renaming
const { formattedFullTime: formattedBirthday } = useFormatDateTime(birthdayDate)
// Confusing "fulltime" for a duration
const { formattedFullTime: formattedEventDuraction } = useFormatDateTime(eventDuration)
// Not obious that formatted time is relative
const { formattedTime: clearAtLabel } = useFormatDateTime(userStatus.clearAt)
/**
* Proposal
*/
const formattedBirthday = useFormatDateTime(birthdayDate)
const formattedEventDuraction = useFormatDateTime(eventDuration)
const clearAtLabel = useRelativeTime(userStatus.clearAt) |
@ShGKme I agree with your points, especially for the functions. Either way its unrelated to this PR, as it is only about functions and those I agree with and will adjust :) |
Do you mean, to have a switch from relative to absolute/duration value? Do you have an example for a use case? It seems to me, relative and general time are always different parts of UI |
// Start the interval for setting the current time if relative time is enabled | ||
onMounted(() => { | ||
if (wrappedOptions.value.relativeTime !== false) { | ||
intervalId = window.setInterval(() => { currentTime.value = Date.now() }, updateInterval.value) | ||
} | ||
}) |
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.
It doesn't require component to be in the DOM (or even to be a component at all)
// Start the interval for setting the current time if relative time is enabled | |
onMounted(() => { | |
if (wrappedOptions.value.relativeTime !== false) { | |
intervalId = window.setInterval(() => { currentTime.value = Date.now() }, updateInterval.value) | |
} | |
}) | |
// Start the interval for setting the current time if relative time is enabled | |
if (wrappedOptions.value.relativeTime !== false) { | |
intervalId = window.setInterval(() => { currentTime.value = Date.now() }, updateInterval.value) | |
} |
// ensure interval is cleared | ||
onUnmounted(() => { | ||
window.clearInterval(intervalId) | ||
}) |
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.
// ensure interval is cleared | |
onUnmounted(() => { | |
window.clearInterval(intervalId) | |
}) | |
// ensure interval is cleared | |
onScopeDispose(() => { | |
if (intervalId !== undefined) { | |
window.clearInterval(intervalId) | |
} | |
}) |
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.
Oh, it is not available in Vue 2
Logreader if the user selects relative time in the settings or "normal time". |
☑️ Resolves
Split the
useFormatDateTime
composable into a general function without reactivity and the composable that uses it.This allows to also use this logic in other places.
🖼️ Screenshots
No visual changes.
🏁 Checklist
next
requested with a Vue 3 upgrade