-
-
Notifications
You must be signed in to change notification settings - Fork 32.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] Remove openPickerIcon
prop in favor of components.OpenPickerIcon
#26223
[pickers] Remove openPickerIcon
prop in favor of components.OpenPickerIcon
#26223
Conversation
openPickerIcon
option to components
prop
openPickerIcon
option to components
propOpenPickerIcon
key to components
prop
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 think that we should drop the openPickerIcon
prop too. It seems to be enough to have it inside the components
prop
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.
Let's discuss this customization approach first: #24511 (comment)
It doesn't seem like we need this customization option.
Thanks for the feedback, i will make the changes over the weekend. |
56448a2
to
b03572a
Compare
b03572a
to
b98a7d1
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.
I have pushed the PR one step forward. The diff in the first custom date time picker example is interesting:
<DateTimePicker
components={{
LeftArrowIcon: AlarmIcon,
RightArrowIcon: SnoozeIcon,
+ OpenPickerIcon: ClockIcon,
}}
- openPickerIcon={<ClockIcon />}
This seems more predictable.
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 don't need this prop: #24511 (comment)
OpenPickerIcon
key to components
propcomponents
prop with OpenPickerIcon
key
components
prop with OpenPickerIcon
keyopenPickerIcon
prop with components
openPickerIcon
prop with components
openPickerIcon
prop
@eps1lon Ok, fair enough, I have changed the lens/framing, isolating the issues and their solutions. |
We are no focusing on a differnt problem, previous review was focused on #24511
openPickerIcon
propopenPickerIcon
prop in favor of components.OpenPickerIcon
Breaking changes
openPickerIcon
prop with thecomponents
prop.Related to #21453 and #24511