Skip to content
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

feat: add today btn #486

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,16 @@
:min="min ? startOfDay(min) : undefined"
:max="max ? endOfDay(max) : undefined"
v-bind="datePickerProps"
:show-adjacent-months="true"
Copy link
Collaborator

@ascott18 ascott18 Oct 25, 2024

Choose a reason for hiding this comment

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

Putting this after v-bind=datePickerProps makes it impossible for users to customize it. It can already be customized by users via datePickerProps, so I don't think this should be enabled by default.

>
<template v-slot:actions>
<v-btn
Copy link
Collaborator

Choose a reason for hiding this comment

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

Button should be on the left, away from the "done" button

@click="setToday"
:class="showTime ? '' : 'mr-10'"
>
Today
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be controllable with a prop, and should be false by default

</v-btn>
</template>
</v-date-picker>

<v-divider vertical></v-divider>
Expand Down Expand Up @@ -160,6 +169,7 @@ import {
startOfDay,
endOfDay,
startOfHour,
set,
} from "date-fns";
import { format, toZonedTime, fromZonedTime } from "date-fns-tz";
import {
Expand Down Expand Up @@ -584,6 +594,34 @@ function emitInput(value: Date | null) {
}
}

function setToday() {
const now = new Date();
const today = startOfDay(now);

// Preserve existing time if it's set
if (internalValueZoned.value) {
const existingDate = internalValueZoned.value;

const newDate = set(today, {
hours: existingDate.getHours(),
minutes: existingDate.getMinutes(),
seconds: existingDate.getSeconds(),
milliseconds: existingDate.getMilliseconds(),
});

if (internalTimeZone.value) {
// Adjust for the specified time zone
modelValue.value = fromZonedTime(newDate, internalTimeZone.value);
} else {
// No time zone specified, use the new date as is
modelValue.value = newDate;
}
Comment on lines +612 to +618
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't sure about this bit, it seems like it's possible we could run into a null time zone... Is it fine to just use the new Date() value in that case?

} else {
// No existing date/time, just set to start of today
modelValue.value = today;
}
Comment on lines +598 to +622
Copy link
Collaborator

Choose a reason for hiding this comment

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

This implementation:

  • Bypasses min/max/allowedDates/step enforcement
  • Doesn't bind to valueOwner (i.e. through :model :for binding)
  • Duplicates time preservation logic

Fortunately, its an easy fix:

Suggested change
const now = new Date();
const today = startOfDay(now);
// Preserve existing time if it's set
if (internalValueZoned.value) {
const existingDate = internalValueZoned.value;
const newDate = set(today, {
hours: existingDate.getHours(),
minutes: existingDate.getMinutes(),
seconds: existingDate.getSeconds(),
milliseconds: existingDate.getMilliseconds(),
});
if (internalTimeZone.value) {
// Adjust for the specified time zone
modelValue.value = fromZonedTime(newDate, internalTimeZone.value);
} else {
// No time zone specified, use the new date as is
modelValue.value = newDate;
}
} else {
// No existing date/time, just set to start of today
modelValue.value = today;
}
dateChanged(new Date())

}

function close() {
menu.value = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
}

.c-time-picker__column {
max-height: 328px;
max-height: 365px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like this is only used for the picker, so figured it would be okay to simply increase this height to deal with the new button. I could also remove showing adjacent months and give the today button negative padding?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It'll need to be conditional on whether the Today button has been enabled or not. c-datetime-picker should own this CSS though, since the adjustment serves the date picker's needs, not the time picker.

overflow-y: auto;
overflow-x: hidden;
padding: 8px 9px;
Expand Down