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

[pickers] Add support for UTC on moment adapter #8031

Merged
merged 1 commit into from
Feb 28, 2023

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Feb 24, 2023

Doc preview

moment.utc does not have the static properties (same issue we had for dayjs).

I think we could update the JSDoc of dateLibInstance to say that it's the instance we use to parse dates, not the instance we use for the rest.

@flaviendelangle flaviendelangle added the component: pickers This is the name of the generic UI component, not the React module! label Feb 24, 2023
@flaviendelangle flaviendelangle self-assigned this Feb 24, 2023
@@ -90,4 +94,12 @@ export class AdapterMoment
public getWeekNumber = (date: defaultMoment.Moment) => {
return date.week();
};

public getWeekdays = () => {
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 think I'll write an RFC in the coming weeks, proposing to finally migrate all the adapters to our repository.
We are duplicating more and more logic...

@mui-bot
Copy link

mui-bot commented Feb 24, 2023

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

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 712.7 1,164.7 716.4 868.12 180.785
Sort 100k rows ms 740.6 1,423 1,311.9 1,126.32 234.909
Select 100k rows ms 239.1 417.1 304.1 303.46 64.645
Deselect 100k rows ms 175.7 323.3 232.6 239.9 53.098

Generated by 🚫 dangerJS against 4e20a13

@flaviendelangle flaviendelangle marked this pull request as ready for review February 24, 2023 10:43
Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

If we reimplement more function, we will also have to test them

Comment on lines +235 to +239
value={moment.utc()}
minDate={moment.utc().startOf('month')}
// ❌ Invalid props
value={moment()}
minDate={moment().startOf('month')}
Copy link
Member

Choose a reason for hiding this comment

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

Adding an empty line could simplify the understanding at first look (without having to stop to think about it)

Suggested change
value={moment.utc()}
minDate={moment.utc().startOf('month')}
// ❌ Invalid props
value={moment()}
minDate={moment().startOf('month')}
value={moment.utc()}
minDate={moment.utc().startOf('month')}
// ❌ Invalid props
value={moment()}
minDate={moment().startOf('month')}

Copy link
Member Author

Choose a reason for hiding this comment

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

Prettier automatically removes it, I agree that it would be nice otherwise

@flaviendelangle flaviendelangle merged commit 7a73e8a into mui:next Feb 28, 2023
@flaviendelangle flaviendelangle deleted the utc-moment branch February 28, 2023 10:36
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!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants