-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[pickers] New unstable field components #5504
Conversation
These are the results for the performance tests:
|
The interactions with arrow keys feel great! Could it somehow block the implementation for a chain of digits as input? |
Btw, where does this POC leave us in terms of a single input implementation for a date range? Would a single input for a range use the same notion used here to isolate the specific 'pieces' of a date? |
I did not look into it yet We also have the date time where we may want to handle it like Retool and open a select, in which case the input must only cover the date part. |
I did not understand sorry |
I raised the question because I noticed you cannot input digits on the current text field POC. It currently only supports the arrow keys, so the question is: Will there be any problem with supporting numeric values as well? And by "chain of digits", I meant if the user could type numbers like: 1 5 0 7 2 0 2 2 to have an output like 15/07/2022 edit: Just saw the latest update to support numeric values, so the question is now partially answered: we can already input numbers! |
Yes, I started to work on it. |
@joserodolfofreitas Concerning the editing, Telerik does not goes from what section of the date to another automatically. If you are on the day and type "1", "2", "3", it will set the day to "1", then "12", then "3". Spectrum does move automatically to the next section. This behavior could be customizable if we have users asking for both |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
For now the placeholder of an empty section is the name of the section ( |
packages/x-date-pickers/src/internals/hooks/useField/useField.ts
Outdated
Show resolved
Hide resolved
I can confirm that all my mentioned issues have been resolved. 🙌 👍 |
@flaviendelangle What do you think about adding the following diff? diff --git a/packages/x-date-pickers/src/internals/hooks/useField/useField.ts b/packages/x-date-pickers/src/internals/hooks/useField/useField.ts
index 5470f19c..84c01798 100644
--- a/packages/x-date-pickers/src/internals/hooks/useField/useField.ts
+++ b/packages/x-date-pickers/src/internals/hooks/useField/useField.ts
@@ -11,12 +11,13 @@ import {
UseFieldState,
} from './useField.interfaces';
import {
+ AvailableAdjustKeyCode,
cleanTrailingZeroInNumericSectionValue,
getMonthsMatchingQuery,
getSectionValueNumericBoundaries,
getSectionVisibleValue,
- incrementDateSectionValue,
- incrementOrDecrementInvalidDateSection,
+ adjustDateSectionValue,
+ adjustInvalidDateSectionValue,
setSectionValue,
} from './useField.utils';
@@ -153,7 +154,7 @@ export const useField = <
}
// Reset the value of the selected section
- case event.key === 'Backspace': {
+ case event.key === 'Backspace' || event.key === 'Delete': {
event.preventDefault();
if (readOnly) {
@@ -181,7 +182,12 @@ export const useField = <
}
// Increment / decrement the selected section value
- case event.key === 'ArrowUp' || event.key === 'ArrowDown': {
+ case event.key === 'ArrowUp' ||
+ event.key === 'ArrowDown' ||
+ event.key === 'Home' ||
+ event.key === 'End' ||
+ event.key === 'PageUp' ||
+ event.key === 'PageDown': {
event.preventDefault();
if (readOnly || state.selectedSectionIndexes == null) {
@@ -196,21 +202,21 @@ export const useField = <
// The date is not valid, we have to increment the section value rather than the date
if (!utils.isValid(activeDate.value)) {
- const newSectionValue = incrementOrDecrementInvalidDateSection(
+ const newSectionValue = adjustInvalidDateSectionValue(
utils,
activeSection,
- event.key === 'ArrowUp' ? 'increment' : 'decrement',
+ event.key as AvailableAdjustKeyCode,
);
updateSections(
setSectionValue(state.sections, state.selectedSectionIndexes.start, newSectionValue),
);
} else {
- const newDate = incrementDateSectionValue(
+ const newDate = adjustDateSectionValue(
utils,
activeDate.value,
activeSection.dateSectionName,
- event.key === 'ArrowDown' ? -1 : 1,
+ event.key as AvailableAdjustKeyCode,
);
const newValue = activeDate.update(newDate);
diff --git a/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts b/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts
index 0dcd3f56..64977a0d 100644
--- a/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts
+++ b/packages/x-date-pickers/src/internals/hooks/useField/useField.utils.ts
@@ -37,33 +37,89 @@ export const getDateSectionNameFromFormat = (format: string): DateSectionName =>
throw new Error(`getDatePartNameFromFormat don't understand the format ${format}`);
};
-export const incrementDateSectionValue = <TDate>(
+export type AvailableAdjustKeyCode =
+ | 'ArrowUp'
+ | 'ArrowDown'
+ | 'PageUp'
+ | 'PageDown'
+ | 'Home'
+ | 'End';
+
+const getDeltaFromKeyCode = (keyCode: Omit<AvailableAdjustKeyCode, 'Home' | 'End'>) => {
+ switch (keyCode) {
+ case 'ArrowUp':
+ return 1;
+ case 'ArrowDown':
+ return -1;
+ case 'PageUp':
+ return 5;
+ case 'PageDown':
+ return -5;
+ default:
+ return 0;
+ }
+};
+
+export const adjustDateSectionValue = <TDate>(
utils: MuiPickersAdapter<TDate>,
date: TDate,
datePartName: DateSectionName,
- datePartValue: 1 | -1,
+ keyCode: AvailableAdjustKeyCode,
) => {
+ const delta = getDeltaFromKeyCode(keyCode);
+ const isStart = keyCode === 'Home';
+ const isEnd = keyCode === 'End';
switch (datePartName) {
case 'day': {
- return utils.addDays(date, datePartValue);
+ if (isStart) {
+ return utils.startOfMonth(date);
+ }
+ if (isEnd) {
+ return utils.endOfMonth(date);
+ }
+ return utils.addDays(date, delta);
}
case 'month': {
- return utils.addMonths(date, datePartValue);
+ if (isStart) {
+ return utils.startOfYear(date);
+ }
+ if (isEnd) {
+ return utils.endOfYear(date);
+ }
+ return utils.addMonths(date, delta);
}
case 'year': {
- return utils.addYears(date, datePartValue);
+ return utils.addYears(date, delta);
}
case 'am-pm': {
- return utils.addHours(date, datePartValue * 12);
+ return utils.addHours(date, delta ? 1 : 1 * 12);
}
case 'hour': {
- return utils.addHours(date, datePartValue);
+ if (isStart) {
+ return utils.startOfDay(date);
+ }
+ if (isEnd) {
+ return utils.endOfDay(date);
+ }
+ return utils.addHours(date, delta);
}
case 'minute': {
- return utils.addMinutes(date, datePartValue);
+ if (isStart) {
+ return utils.setMinutes(date, 0);
+ }
+ if (isEnd) {
+ return utils.setMinutes(date, 59);
+ }
+ return utils.addMinutes(date, delta);
}
case 'second': {
- return utils.addSeconds(date, datePartValue);
+ if (isStart) {
+ return utils.setSeconds(date, 0);
+ }
+ if (isEnd) {
+ return utils.setSeconds(date, 59);
+ }
+ return utils.addSeconds(date, delta);
}
default: {
return date;
@@ -277,13 +333,16 @@ export const getSectionValueNumericBoundaries = <TDate>(
}
};
-export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends FieldSection>(
+export const adjustInvalidDateSectionValue = <TDate, TSection extends FieldSection>(
utils: MuiPickersAdapter<TDate>,
section: TSection,
- type: 'increment' | 'decrement',
+ keyCode: AvailableAdjustKeyCode,
) => {
const today = utils.date()!;
- const delta = type === 'increment' ? 1 : -1;
+ const delta = getDeltaFromKeyCode(keyCode);
+ const isStart = keyCode === 'Home';
+ const isEnd = keyCode === 'End';
+ const shouldSetAbsolute = section.value === '' || isStart || isEnd;
switch (section.dateSectionName) {
case 'year': {
@@ -299,8 +358,8 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
case 'month': {
let newDate: TDate;
- if (section.value === '') {
- if (type === 'increment') {
+ if (shouldSetAbsolute) {
+ if (delta > 0 || isEnd) {
newDate = utils.endOfYear(today);
} else {
newDate = utils.startOfYear(today);
@@ -314,8 +373,8 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
case 'day': {
let newDate: TDate;
- if (section.value === '') {
- if (type === 'increment') {
+ if (shouldSetAbsolute) {
+ if (delta > 0 || isEnd) {
newDate = utils.endOfMonth(today);
} else {
newDate = utils.startOfMonth(today);
@@ -332,7 +391,7 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
const pm = utils.formatByString(utils.endOfDay(today), section.formatValue);
if (section.value === '') {
- if (type === 'increment') {
+ if (delta > 0 || isEnd) {
return pm;
}
return am;
@@ -347,8 +406,8 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
case 'hour': {
let newDate: TDate;
- if (section.value === '') {
- if (type === 'increment') {
+ if (shouldSetAbsolute) {
+ if (delta > 0 || isEnd) {
newDate = utils.endOfDay(today);
} else {
newDate = utils.startOfDay(today);
@@ -362,9 +421,9 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
case 'minute': {
let newDate: TDate;
- if (section.value === '') {
+ if (shouldSetAbsolute) {
// TODO: Add startOfHour and endOfHours to adapters to avoid hard-coding those values
- const newNumericValue = type === 'increment' ? 59 : 0;
+ const newNumericValue = delta > 0 || isEnd ? 59 : 0;
newDate = utils.setMinutes(today, newNumericValue);
} else {
newDate = utils.addMinutes(utils.setMinutes(today, Number(section.value)), delta);
@@ -375,9 +434,9 @@ export const incrementOrDecrementInvalidDateSection = <TDate, TSection extends F
case 'second': {
let newDate: TDate;
- if (section.value === '') {
+ if (shouldSetAbsolute) {
// TODO: Add startOfMinute and endOfMinute to adapters to avoid hard-coding those values
- const newNumericValue = type === 'increment' ? 59 : 0;
+ const newNumericValue = delta > 0 || isEnd ? 59 : 0;
newDate = utils.setSeconds(today, newNumericValue);
} else {
newDate = utils.addSeconds(utils.setSeconds(today, Number(section.value)), delta);
|
I applied @LukasTy suggestion to handle other keys edition |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Maybe move it to the issue #5531, it seems to be a good place to discuss cross PR topics around the fields. |
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 looks great! I have left a few comments as code comments so we can benefit from threads (#5531) doesn't provide this.
|
||
return ( | ||
<LocalizationProvider dateAdapter={AdapterDateFns}> | ||
<Stack spacing={2} sx={(theme) => ({ width: theme.spacing(48) })}> |
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 might make the demo easier to scan:
<Stack spacing={2} sx={(theme) => ({ width: theme.spacing(48) })}> | |
<Stack spacing={2} sx={{ width: 384 })}> |
export default function BasicDateField() { | ||
return ( | ||
<LocalizationProvider dateAdapter={AdapterDateFns}> | ||
<DateField label="Basic date field" /> |
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.
End-users need to click twice to be able to select the year. Compare:
Screen.Recording.2022-08-24.at.22.33.21.mov
https://deploy-preview-5504--material-ui-x.netlify.app/x/react-date-pickers/date-field/#basic-usage
With (better):
Screen.Recording.2022-08-24.at.22.35.41.mov
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/date
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 one is a known issue
You have the same in Telerik: https://www.telerik.com/kendo-react-ui/components/dateinputs/datepicker/
The problem is that I select all sections on focus.
And when you click, it fires onFocus before onClick.
So when onClick is fired, the focus the selection caused by the focus is already applied, so it clicks on a fully selected input.
And on a fully selected input, I can't find the cursor position of the click, the cursor just moves to the beginning of the input.
One solution would be to delay the selection caused by the focus and to skip it if a click selection has been done in the meanwhile.
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.
I select all sections on focus
I would dive into this. I think it's where the problem starts:
mui-x/packages/x-date-pickers/src/internals/hooks/useField/useField.ts
Lines 319 to 322 in 8db0bfa
const handleInputFocus = useEventCallback(() => { | |
// TODO: Avoid applying focus when focus is caused by a click | |
updateSelectedSections(0, state.sections.length - 1); | |
}); |
Maybe instead of a "select all" on focus, the browser does it automatically when focusing an input with a Tab, we could look at inputRef.current.selectionEnd - inputRef.current.selectionStart
. If the value equals 0 it would mean that it wasn't a keyboard focus, it was a click focus with a caret that has no selections. Then, we can look at where the current selection is on, and select the corresponding segment.
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.
I have a PR to improve this behavior #5901
<DateField | ||
label="French locale (default format)" | ||
value={value} | ||
onChange={(newValue) => setValue(newValue)} | ||
/> | ||
<DateField | ||
label="French locale (full letter month)" | ||
value={value} | ||
onChange={(newValue) => setValue(newValue)} | ||
format="dd MMMM yyyy" | ||
/> |
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.
I agree with #5531 (comment). We might want to change the placeholder. Say the input is empty:
Can you guess that it should look like this in the end?
https://deploy-preview-5504--material-ui-x.netlify.app/x/react-date-pickers/date-field/#localization
It might be clearer like this for the first textbox:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/date
The second advantage is that it would minimize layout shifts.
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.
export default function BasicDateField() { | ||
return ( | ||
<LocalizationProvider dateAdapter={AdapterDateFns}> | ||
<DateField label="Basic date field" /> |
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 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.
I don't know for this one
In Spectrum, if I press 0
it does not write anything, even if the end format will be 01
Press 0
=> still render the placeholder (totally ignores the event I think)
Press 1
=> renders 01
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.
Should we allow 00?
To be more precise, I was thinking of what would happen when leaving the selection. A native browser seems to turn 00 to 01 when unselecting.
import * as React from 'react'; | ||
import TextField from '@mui/material/TextField'; | ||
|
||
import { DateFieldProps } from './DateField.interfaces'; | ||
import { useDateField } from './useDateField'; | ||
|
||
type DateFieldComponent = (<TInputDate, TDate = TInputDate>( | ||
props: DateFieldProps<TInputDate, TDate> & React.RefAttributes<HTMLInputElement>, | ||
) => JSX.Element) & { propTypes?: any }; | ||
|
||
export const DateField = React.forwardRef(function DateField<TInputDate, TDate = TInputDate>( | ||
inProps: DateFieldProps<TInputDate, TDate>, | ||
ref: React.Ref<HTMLInputElement>, | ||
) { | ||
const { inputRef, inputProps } = useDateField< | ||
TInputDate, | ||
TDate, | ||
DateFieldProps<TInputDate, TDate> | ||
>(inProps); | ||
|
||
return <TextField ref={ref} inputRef={inputRef} {...inputProps} />; | ||
}) as DateFieldComponent; |
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.
Looking at the simplicity of the code, I almost feel like it would be better to expose this userland. Not sure.
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 is exposed and should be documented soon
Do you think we should not expose the component ?
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.
OK great to know.
Do you think we should not expose the component ?
I can't make my mind on this. I think that it's not impossible that we could be better off without this abstraction, with the rationale that lower level API makes easier customizations.
As a side note, I wonder if developers won't expect to be able to target MuiDateField
in the theme.
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.
https://github.com/mui/mui-x/blob/next/packages/x-date-pickers/src/DateField/DateField.tsx
The component has a little more logic know and supports theme overrides as you mentionned.
And we have multi-input range fields with a lot more logic (those I would clearly not remove them).
These components are very early stage, please don't use them in production
Doc Preview of DateField
Doc Preview of DateRangeField
Part of #5531
Wanted interactions
Missing
@date-io
methodsaddYear
(similar toaddDay
andaddMonth
) => feat: New methodaddYear
dmtrKovalenko/date-io#623