WIP - [PER-10416] Add date and time selection modal#924
Draft
aasandei-vsp wants to merge 7 commits intomainfrom
Draft
WIP - [PER-10416] Add date and time selection modal#924aasandei-vsp wants to merge 7 commits intomainfrom
aasandei-vsp wants to merge 7 commits intomainfrom
Conversation
aasandei-vsp
force-pushed
the
PER-10416-new-date-editing
branch
from
77aad83 to
adb771a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #924 +/- ##
==========================================
+ Coverage 48.69% 50.10% +1.40%
==========================================
Files 350 359 +9
Lines 11339 11956 +617
Branches 1898 2104 +206
==========================================
+ Hits 5522 5991 +469
- Misses 5627 5735 +108
- Partials 190 230 +40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
aasandei-vsp
force-pushed
the
PER-10416-new-date-editing
branch
2 times, most recently
from
e5ac965 to
f4748a1
Compare
Member
cecilia-donnelly
left a comment
cecilia-donnelly
left a comment
There was a problem hiding this comment.
I'm sorry, I forgot to finish and submit this!
| day: string; | ||
| } | ||
|
|
||
| const DAYS_IN_MONTH = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; |
Member
There was a problem hiding this comment.
It seems wrong for us to keep track of this ourselves - is there a way to use a library?
| if (!m || m < 1 || m > 12) return 31; | ||
|
|
||
| if (m === 2 && y) { | ||
| return this.isLeapYear(y) ? 29 : 28; |
Member
There was a problem hiding this comment.
I see you handle February here, but it does seem strange to have to write this ourselves.
|
|
||
| endTime = signal<TimeObject>({ ...DEFAULT_TIME }); | ||
|
|
||
| edtfValue = computed(() => { |
Member
There was a problem hiding this comment.
EDTF can get complicated - I would use something like https://www.npmjs.com/package/edtf?activeTab=readme for parsing
Member
There was a problem hiding this comment.
That one might be better suited to transforming a stored EDTF date back into something that makes sense on the client? Generally just saying that we don't need to write all the logic ourselves!
aasandei-vsp
force-pushed
the
PER-10416-new-date-editing
branch
3 times, most recently
from
543c7d2 to
373b5ca
Compare
This new component will be presentational, it will get a date input and it will also return a selected date. It includes a calendar. Issue: PER-10416
The time picker is a presentational component, its input si a time object and it returns a time object as well. It contains a time selector. It works with meridian. Issue: PER-10416
The edit date and time component is smart, because it manages its own state, but also the states of the time and date pickers. But still the state is isolated to this specific component, it gets as input a date and time object and returns back a date and time object. It handles the time picker, the date picker, the date qualifiers, start and end time and also shows an edtf calculated value. Issue: PER-10416
The service has the resposability to open the modal for the edit date time component and provide the initial data. Issue: PER-10416
Because we are using the timezone dropdown component in three places now, extracting it into a presentational component for reusability seems the right approach. It will be a generic isolated component that will provide an input and a dropdown for choosing the timezone. Issue: PER-10416
aasandei-vsp
force-pushed
the
PER-10416-new-date-editing
branch
from
373b5ca to
400b7dc
Compare
This component will be the smaller version to set the date and time, it will open the modal editor for more comporehensive choices. Issue: PER-10416
aasandei-vsp
force-pushed
the
PER-10416-new-date-editing
branch
from
400b7dc to
db36d72
Compare
Issue: PER-10416
aasandei-vsp
changed the title
aasandei-vsp
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.