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

[docs] Add full custom field creation example #15194

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Oct 30, 2024

Part of #14496
Doc preview

This is the end of the planned work for custom field with custom editing behavior.
I'll focus on the other v8 topics now 👍

@flaviendelangle flaviendelangle marked this pull request as draft October 30, 2024 11:39
@mui-bot
Copy link

mui-bot commented Oct 30, 2024

@flaviendelangle flaviendelangle added docs Improvements or additions to the documentation component: pickers This is the name of the generic UI component, not the React module! labels Oct 30, 2024
@flaviendelangle flaviendelangle self-assigned this Oct 30, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Oct 31, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 31, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 13, 2024
@flaviendelangle flaviendelangle added the on hold There is a blocker, we need to wait label Nov 13, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2024
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2024
@flaviendelangle flaviendelangle removed the on hold There is a blocker, we need to wait label Nov 14, 2024
@flaviendelangle flaviendelangle marked this pull request as ready for review November 14, 2024 09:25
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Looks great! 💯
Awesome improvement from the previous experience! 🚀 💙

Nitpick: Have you considered using input element in the examples, further signaling that you can customize it to your heart's content? 🤔

@@ -154,3 +154,309 @@ and you don't want the UI to look like a Text Field, you can replace the field w
The same logic can be applied to any Range Picker:

{{"demo": "behavior-button/MaterialDateRangePicker.js", "defaultCodeOpen": false}}

### Build your own custom field
Copy link
Member

Choose a reason for hiding this comment

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

WDYT about moving all these headings one level up (i.e. making this a separate section, where all subsections would end up in the side menu... 🤔 )?

Copy link
Member Author

Choose a reason for hiding this comment

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

I did that at first
And then moved it to be inside "With a custom editing experience" because it's what it describes (not how to build a custom field that has the built in editing experience).
But both make sense and have different benefits.

If you prefer the standalone h2, I can do it 👍

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. My only gripe is that the section is lengthy, but there is no way to link to specific parts via headers. 🤔
I have no strong preference here, whatever you think works best. 👍

```jsx
function CustomDateField(props) {
const { format, value, onChange } = props;
const [inputValue, setInputValue] = useInputValue(value, format);
Copy link
Member

Choose a reason for hiding this comment

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

My fear of putting so much fragile and important code in MD is the risk of forgetting to update it when the API changes. 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not optimal indeed
How would you handle it?
My goal is to decompose the code to make it easier to read.

But maybe I can be more abstracted in the code snippets.
With things like:

To access the format, you can use the `usePickerContext()` hook:

```ts
import { usePickerContext } from '@mui/x-date-pickers/hook';

const { fieldFormat } = usePickerContext();
```

(I need to update this PR after #16042 btw)

And then in the final example I put a lot of comment in the code to explain what each part does.

Copy link
Member

Choose a reason for hiding this comment

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

How would you handle it?

Yeah, one option is to minimize the amount of code as you suggested.
Or we could try and see if it could be authored in a file and be used as

{{ "component": "HookBuildingExample.ts" }}

in md, but I'm not sure if we can use ts files in such cases. 🙈

docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
docs/data/date-pickers/custom-field/custom-field.md Outdated Show resolved Hide resolved
const { internalProps, forwardedProps } = useSplitFieldProps(other, 'date');

return (
<TextField {...forwardedProps} value={inputValue} onChange={handleChange}>
Copy link
Member

Choose a reason for hiding this comment

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

Great addition, but WDYT about using <input here to give a bit more precise example (TextField does support those concepts)?

Suggested change
<TextField {...forwardedProps} value={inputValue} onChange={handleChange}>
<input {...forwardedProps} value={inputValue} onChange={handleChange}>

Comment on lines +417 to +460
##### Updated example

Here is the updated example with the props forwarded to the DOM:

```jsx
function CustomDateField(props) {
// TextField does not support slots and slotProps before `@mui/material` v6.0
const { slots, slotProps, ...other } = props;
const { internalProps, forwardedProps } = useSplitFieldProps(other, 'date');

const { format, timezone, value, onChange } = internalProps;
const [inputValue, setInputValue] = useInputValue(value, format);

const { hasValidationError, getValidationErrorForNewValue } = useValidation({
value,
timezone,
props: internalProps,
validator: validateDate,
});

const handleChange = (event) => {
const newInputValue = event.target.value;
const newValue = dayjs(newInputValue, format);
setInputValue(newInputValue);
onChange(newValue, { validationError: getValidationErrorForNewValue(newValue) });
};

const placeholder = useParsedFormat(internalProps);

return (
<TextField
{...forwardedProps}
placeholder={placeholder}
value={inputValue}
onChange={handleChange}
error={hasValidationError}
/>
);
}
```

#### Enhanced basic example

Here is a live demo of the example created in all the previous sections:
Copy link
Member

Choose a reason for hiding this comment

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

WFYT about avoiding duplication here? 🤔
I feel like having the final live example here should be enough. 🤞

Besides, both section headings sound a bit misaligned with the presented information. 🙈
Maybe we could stick with just one heading, i.e.: Full live example or Full custom example.

@LukasTy LukasTy changed the title [docs] Add proper documentation on how to create custom fields [docs] Add full custom fields creation example Jan 8, 2025
@LukasTy LukasTy changed the title [docs] Add full custom fields creation example [docs] Add full custom field creation example Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pickers This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants