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.

Sign up for GitHub

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

Jump to bottom

[docs] New page for TimeClock #7280

Merged
merged 7 commits into from
Dec 29, 2022
Merged

[docs] New page for TimeClock #7280

merged 7 commits into from
Dec 29, 2022

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Dec 21, 2022 •
edited
Loading

Doc preview

What's next ?

  • Date Range Calendar page
  • Cleanup of the Date Range Picker and Date Time Picker pages

@flaviendelangle flaviendelangle self-assigned this Dec 21, 2022
@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 Dec 21, 2022
flaviendelangle
flaviendelangle commented Dec 21, 2022
View reviewed changes
packages/x-date-pickers/src/TimeClock/TimeClock.tsx

const props = useThemeProps({
props: inProps,
name: 'MuiTimeClock',
});

const {
ampm = false,
ampm = utils.is12HourCycleInCurrentLocale(),
Copy link
Member Author

@flaviendelangle flaviendelangle Dec 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I added a smart fallback to be consistent with the picker behavior
I can extract this breaking change to a standalone PR if you want.

Note that it's a breaking change only for direct usage of TimeClock, not for TimePicker and DateTimePicker

Copy link
Member

Choose a reason for hiding this comment

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

I'd say just go with adding it in this PR.
I reckon that barely anyone was using the standalone Clock component.
But I do still think that we should include a breaking change item in migration guide as well as a changelog entry. The example should probably reference the original component name (was it Clock?) as well..? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

The old name was ClockPicker
I'll add it to the migration guide 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

@mui-bot
Copy link

mui-bot commented Dec 21, 2022 •
edited
Loading

Messages
📖 Netlify deploy preview: https://deploy-preview-7280--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 668.9 1,115.5 685 855.74 169.124
Sort 100k rows ms 648.8 1,206.6 1,206.6 965.28 230.434
Select 100k rows ms 200.9 399.5 303.7 298.5 79.754
Deselect 100k rows ms 162 283.9 208.4 222.3 45.156

Generated by 🚫 dangerJS against c5092df

@flaviendelangle flaviendelangle marked this pull request as ready for review December 21, 2022 16:18
LukasTy
LukasTy approved these changes Dec 22, 2022
View reviewed changes
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.

LGTM, just agreeing with the breaking change and so it would be great to update migration guide at the same time. 👌

packages/x-date-pickers/src/TimeClock/TimeClock.tsx

const props = useThemeProps({
props: inProps,
name: 'MuiTimeClock',
});

const {
ampm = false,
ampm = utils.is12HourCycleInCurrentLocale(),
Copy link
Member

Choose a reason for hiding this comment

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

I'd say just go with adding it in this PR.
I reckon that barely anyone was using the standalone Clock component.
But I do still think that we should include a breaking change item in migration guide as well as a changelog entry. The example should probably reference the original component name (was it Clock?) as well..? 🤔

@flaviendelangle flaviendelangle merged commit 1ebeb95 into mui:next Dec 29, 2022
@flaviendelangle flaviendelangle deleted the time-clock-doc branch December 29, 2022 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukasTy LukasTy LukasTy approved these changes

Assignees

@flaviendelangle flaviendelangle

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@flaviendelangle @mui-bot @LukasTy