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] Introduce a new concept of manager #15339

Merged
merged 86 commits into from
Dec 31, 2024
Merged

[pickers] Introduce a new concept of manager #15339

merged 86 commits into from
Dec 31, 2024

Conversation

flaviendelangle
Copy link
Member

@flaviendelangle flaviendelangle commented Nov 8, 2024 •
edited
Loading

Part of #15395

You can look at #15505 to see the benefit of this approach in a concrete use case.

In this PR

  • Create the interfaces for the new manager
  • Create the hooks to create all the managers (date, time, date time, date range, time range and date time range)

Todo after this PR

  • Stop duplicating logic in the multi input range field ([pickers] Remove code duplication for the multi input range fields #15505)
  • Rework useField to only have two parameters (manager and props)
  • Move the application of the default props inside useField instead of inside useDateField & co
  • Extend the new manager to work with the picker (the exact needs are unclear for now, we should be able to move the application of default props inside it like for the field)
  • Stop using useDateField & co in DateField & co and instead directly use the manager

@flaviendelangle flaviendelangle self-assigned this Nov 8, 2024
@mui-bot
Copy link

mui-bot commented Nov 8, 2024 •
edited
Loading

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

Generated by 🚫 dangerJS against 245c08f

@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 12, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 12, 2024
@flaviendelangle flaviendelangle force-pushed the v8-value-manager branch 2 times, most recently from 01934b9 to 1390421 Compare November 13, 2024 08:18
@flaviendelangle flaviendelangle force-pushed the v8-value-manager branch 2 times, most recently from f1e96db to 8592c72 Compare November 13, 2024 09:34
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 28, 2024
@flaviendelangle flaviendelangle marked this pull request as ready for review November 28, 2024 15:17
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Dec 9, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

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

Awesome work and really neat idea for the manager! 👏 💯
The final DX result looks very enticing. 💙

packages/x-date-pickers/src/models/manager.ts
* Object containing basic methods to interact with the value of the picker or field.
* This property is not part of the public API and should not be used directly.
*/
internal_valueManager: PickerValueManager<TValue, TError>;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Have you considered using a Class with private properties? 🤔

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 to access the properties outside of the class so I don't think that would be feasible 😬

@LukasTy LukasTy added the new feature New feature or request label Dec 31, 2024
@flaviendelangle flaviendelangle merged commit 20e037b into mui:master Dec 31, 2024
18 checks passed
@flaviendelangle flaviendelangle mentioned this pull request Jan 2, 2025
Draft
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

@noraleonte noraleonte Awaiting requested review from noraleonte

Assignees

@flaviendelangle flaviendelangle

Labels
component: pickers This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

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