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

[pickers] Remove TSection and strictly type TValue #15434

Merged
merged 35 commits into from
Nov 28, 2024
Merged

[pickers] Remove TSection and strictly type TValue #15434

merged 35 commits into from
Nov 28, 2024

Conversation

flaviendelangle
Copy link
Member

Alternative DX discussed with @LukasTy in #15290

Part of #14823 (step 2)

I will handle the migration guide in a follow up 👍

Example

 UseFieldInternalProps<
  PickerRangeValue,
-  RangeFieldSection,
   TEnableAccessibleFieldDOMStructure,
   TimeRangeValidationError
 >

@mui-bot
Copy link

mui-bot commented Nov 15, 2024 •
edited
Loading

Deploy preview: https://deploy-preview-15434--material-ui-x.netlify.app/

Updated pages:

Generated by 🚫 dangerJS against 2823188

@flaviendelangle flaviendelangle self-assigned this Nov 15, 2024
@flaviendelangle flaviendelangle added breaking change component: pickers This is the name of the generic UI component, not the React module! labels Nov 15, 2024
@flaviendelangle flaviendelangle force-pushed the tvalue branch 2 times, most recently from 627848d to 627900d Compare November 15, 2024 13:10
@flaviendelangle flaviendelangle marked this pull request as ready for review November 15, 2024 13:19
flaviendelangle
flaviendelangle commented Nov 15, 2024
View reviewed changes
packages/x-date-pickers/src/internals/hooks/usePicker/usePickerValue.types.ts
/**
* Determines if two values are equal.
* @template TValue
* @template TValue The value type. It will be the same type as `value` or `null`. It can be in `[start, end]` format in case of range value.
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'm not a huge fan of this template.
I had the same everywhere for TIsRange so when I moved to the new TValue approached I just changes it everywhere, I can improve the sentence in a follow up.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that it can be improved. 👍
But I also see value in the information it portrays. 🤔

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 we could improve the wording, but this one is better than nothing for sure

packages/x-date-pickers-pro/src/models/index.ts
@@ -3,3 +3,5 @@ export * from './fields';
export * from './range';
export * from './validation';
export * from './multiInputRangeFieldClasses';

export type { RangePosition } from '@mui/x-date-pickers/internals';
Copy link
Member Author

Choose a reason for hiding this comment

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

We need the type on the community package, but as internal, and we make it public on the pro package

@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 19, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 19, 2024
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 20, 2024
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 21, 2024
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 22, 2024
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 22, 2024
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 26, 2024
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 26, 2024
LukasTy
LukasTy approved these changes Nov 28, 2024
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.

Nice cleanup, great job! 👍 💯
Leaving a few nitpicks. 😉

docs/data/migration/migration-pickers-v7/migration-pickers-v7.md Outdated Show resolved Hide resolved
packages/x-date-pickers/src/DateCalendar/tests/describes.DateCalendar.test.tsx Show resolved Hide resolved
packages/x-date-pickers/src/internals/hooks/useClockReferenceDate.ts Outdated Show resolved Hide resolved
packages/x-date-pickers/src/MultiSectionDigitalClock/MultiSectionDigitalClock.types.ts Show resolved Hide resolved
packages/x-date-pickers/src/internals/hooks/usePicker/usePickerValue.types.ts
/**
* Determines if two values are equal.
* @template TValue
* @template TValue The value type. It will be the same type as `value` or `null`. It can be in `[start, end]` format in case of range value.
Copy link
Member

Choose a reason for hiding this comment

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

I agree that it can be improved. 👍
But I also see value in the information it portrays. 🤔

packages/x-date-pickers/src/internals/utils/date-utils.ts Show resolved Hide resolved
flaviendelangle and others added 4 commits November 28, 2024 13:43
@flaviendelangle @LukasTy
Update docs/data/migration/migration-pickers-v7/migration-pickers-v7.md
e47e9fd 
Co-authored-by: Lukas Tyla <llukas.tyla@gmail.com>
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
@flaviendelangle
Merge branch 'master' into tvalue
4452039
@flaviendelangle
Fix
e5406e7
@flaviendelangle
Merge branch 'master' into tvalue
2823188
@flaviendelangle flaviendelangle merged commit 4108634 into mui:master Nov 28, 2024
18 checks passed
LukasTy added a commit to LukasTy/mui-x that referenced this pull request Dec 19, 2024
@flaviendelangle @LukasTy
[pickers] Remove TSection and strictly type TValue (mui#15434)
7c8fbcd 
Signed-off-by: Flavien DELANGLE <flaviendelangle@gmail.com>
Co-authored-by: Lukas Tyla <llukas.tyla@gmail.com>
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
breaking change component: pickers This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

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