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 example to add a second icon next to the field's opening button #12524

Merged
merged 5 commits into from Mar 22, 2024

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Mar 21, 2024

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.

@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 Mar 21, 2024
@flaviendelangle flaviendelangle self-assigned this Mar 21, 2024
@mui-bot
Copy link

mui-bot commented Mar 21, 2024

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.

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
Copy link
Member

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. 🤔

Suggested change
## Add a second icon next to the opening button
## Add an icon next to the opening button

Copy link
Member Author

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.

Copy link
Member

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? 🤔

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link
Member

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. 👌

@flaviendelangle
Copy link
Member Author

WDYT, would it make sense to also mention that applies to any Picker component?

We are not mentionning it on the other examples of this page, but maybe we should

@flaviendelangle flaviendelangle merged commit 02867d1 into mui:master Mar 22, 2024
17 checks passed
@flaviendelangle flaviendelangle deleted the additional-icon branch March 22, 2024 12:42
@flaviendelangle flaviendelangle restored the additional-icon branch March 25, 2024 07:09
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.

[pickers] Support additional endAdornment on <DatePicker />
3 participants