-
Notifications
You must be signed in to change notification settings - Fork 65
Replace masked TimeCell input with section-based state machine #14822
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: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Pivouane (Pierre Santamaria) <pierrenumber1@outlook.com>
8cc6c80 to
f3fa397
Compare
Signed-off-by: Pivouane (Pierre Santamaria) <pierrenumber1@outlook.com>
Signed-off-by: Pivouane (Pierre Santamaria) <pierrenumber1@outlook.com>
kmer2016
left a comment
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.
Well done ! Just some nit comment.
Integration changes will likely be needed when connecting to backend via PR #14853.
| const toSectionState = (str: string, section: Section): SectionState => { | ||
| const digits = str.replace(/\D/g, '').slice(-2); | ||
| if (digits.length === 2) return digits; | ||
| const sectionPlaceholder = SECTION_PLACEHOLDERS[section]; | ||
| if (digits.length === 1) return sectionPlaceholder + digits; | ||
| return sectionPlaceholder + sectionPlaceholder; | ||
| }; |
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.
We can moving the placeholder variable to the top for clearer separation between setup and conditions:
| const toSectionState = (str: string, section: Section): SectionState => { | |
| const digits = str.replace(/\D/g, '').slice(-2); | |
| if (digits.length === 2) return digits; | |
| const sectionPlaceholder = SECTION_PLACEHOLDERS[section]; | |
| if (digits.length === 1) return sectionPlaceholder + digits; | |
| return sectionPlaceholder + sectionPlaceholder; | |
| }; | |
| const formatSectionDigits = (str: string, section: Section): SectionState => { | |
| const digits = str.replace(/\D/g, '').slice(-2); | |
| const sectionPlaceholder = SECTION_PLACEHOLDERS[section]; | |
| if (digits.length === 2) return digits; | |
| if (digits.length === 1) return sectionPlaceholder + digits; | |
| return sectionPlaceholder + sectionPlaceholder; | |
| }; |
| const getSectionStateDigitCount = (sectionState: SectionState): number => { | ||
| const digitCount = sectionState.replace(/\D/g, '').length; | ||
| return digitCount; | ||
| }; |
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.
nit:
| const getSectionStateDigitCount = (sectionState: SectionState): number => { | |
| const digitCount = sectionState.replace(/\D/g, '').length; | |
| return digitCount; | |
| }; | |
| const countDigits = (sectionState: SectionState): number => sectionState.replace(/\D/g, '').length; |
| return digitCount; | ||
| }; | ||
|
|
||
| const isFull = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 2; |
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.
nit
| const isFull = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 2; | |
| const hasAllDigits = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 2; |
|
|
||
| const isFull = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 2; | ||
|
|
||
| const isEmpty = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 0; |
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.
nit:
| const isEmpty = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 0; | |
| const hasNoDigits = (pair: SectionState): boolean => getSectionStateDigitCount(pair) === 0; |
Signed-off-by: Pivouane (Pierre Santamaria) <pierrenumber1@outlook.com>
Context
The previous
TimeCellimplementation relied on a input mask and tweaks with a microTask queue.This was not a good solution for multiple reasons, including complexity.
What changed
The TimeCell now works with a section-based approach:
hours,minutes,seconds)How should this work
This refactor also focuses on behaviors implied by the UX/UI requirements, to ensure that this sketch is respected here's a few test cases for those who needs to review the component.
0145while focused on minutes and pressing enter should display01:45:00Rewriting the integration of the timeCell also accomodates the work of linking the table with the backend process.
related to https://github.com/osrd-project/osrd-confidential/issues/1281
This closes #14277 and closes #14275
because it will most likely be handled by the backend manipulations.