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 example to add a second icon next to the field's opening button #12524
Conversation
Deploy preview: https://deploy-preview-12524--material-ui-x.netlify.app/ Updated pages: |
4d17ed2
to
b5eb832
Compare
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.
Nice improvement! 💯 👍
WDYT, would it make sense to also mention that applies to any Picker component? 🤔
@@ -34,3 +34,10 @@ If you want to track the opening of the picker, you should use the `onOpen` / `o | |||
``` | |||
|
|||
::: | |||
|
|||
## Add a second icon next to the opening button |
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.
Making it shorter 🙈
It could also apply to the range picker and in that case, it wouldn't be the second icon, at least with the current implementation. 🤔
## Add a second icon next to the opening button | |
## Add an icon next to the opening button |
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.
Great point
I added a demo for the range picker and we can't use the inputAdornment
slot their because we don't have it, instead we can use textField.InputProps.endAdornment
.
I'm not a fan of having 2 different DX depending on the component but I don't see how to solve this.
On the non-range picker I can't use textField.InputProps.endAdornment
because we would loose the icon to open the picker AND the clearable icon.
The same is probably also true for the mobile... this topic might be harder than anticipated 😆
I'm wondering it we shouldn't just add the inputAdornment
on every picker and just have it as null
by default on the one that don't have any icon rendered.
That way the customization DX is easier, including for the SingleInputDateRangePickerWithAdornment
on the "Custom field" page.
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 just shows that we clearly need more alignment in this regard and implementing the open button at least on Single Range Pickers. 🤔
Anyway, that's a story for later time. 👍
But on the other hand, WDYT, does it even make sense to have an inputAdornment
slot?
If it can be done via textField.InputProps.endAdornment
, why not try making it the only source of truth? 🤔
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.
One behavior change (I would not call that a breaking change) is that passing a slots.inputAdornment
would now also impact mobile when using a responsive picker.
Maybe passing a variable in the owner state saying if we are on mobile or on desktop would be nice to allow to customize both independently.
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.
If it can be done via textField.InputProps.endAdornment, why not try making it the only source of truth?
Because, at least with the current implementation, you can access the children of the input adornment that we are passing, so you can't add something next to it without loosing the initial one.
I do agree that we need to clean this API, but it will probably be breaking in some way if we want to do it properly.
Maybe we can start by documenting what can be done with the current API, and create an issue to explore the best DX we can find to do all the customization around the adornments.
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.
Maybe passing a variable in the owner state saying if we are on mobile or on desktop would be nice to allow to customize both independently.
Great suggestion. 👍
WDYT about firstly waiting for a validation if this is requested? Maybe before that we might have an open button mobile as well? 🙈 😆
Maybe we can start by documenting what can be done with the current API, and create an issue to explore the best DX we can find to do all the customization around the adornments.
Sure, it makes the most sense. 👌
docs/data/date-pickers/custom-opening-button/custom-opening-button.md
Outdated
Show resolved
Hide resolved
We are not mentionning it on the other examples of this page, but maybe we should |
5692017
to
0a377fa
Compare
0a377fa
to
d8a7de3
Compare
Closes #11425
Doc preview
This demo is really about the opening button, but I think this is the page people are the most likely to go looking for it.
If I add it to Custom field it would be lost in a very long list of examples.