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

[DateTimeRangePicker] Change OK button behavior #11829

Open
joserodolfofreitas opened this issue Jan 27, 2024 · 5 comments
Open

[DateTimeRangePicker] Change OK button behavior #11829

joserodolfofreitas opened this issue Jan 27, 2024 · 5 comments
Assignees
Labels
breaking change component: DateTimeRangePicker The React component component: pickers This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature v8.x

Comments

@joserodolfofreitas
Copy link
Member

joserodolfofreitas commented Jan 27, 2024

Steps to reproduce

Link to live example: (required)

Steps:

  1. go to basic example
  2. input the first calendar date and click ok
Screen.Recording.2024-01-27.at.08.01.30.mov

Current behavior

If users click the OK button on the start input, it closes the view.

Expected behavior

If users click the OK button on the start input, it should move to the end input.

Context

No response

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: datetimerangepicker okbutton

@joserodolfofreitas joserodolfofreitas added status: waiting for maintainer These issues haven't been looked at yet by a maintainer bug 🐛 Something doesn't work component: DateTimeRangePicker The React component and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jan 27, 2024
@gitstart
Copy link
Contributor

gitstart commented Feb 1, 2024

@joserodolfofreitas we would like to pick this up

@joserodolfofreitas
Copy link
Member Author

Thank you for offering to take on this issue!
We really appreciate your willingness to contribute. However, this particular issue can be quite deceiving, in the sense that it looks simple, but it's fairly complex and require deep understanding of the component.

We recommend getting started on issues labeled with "good first issue".
https://github.com/mui/mui-x/issues?q=is%3Aissue+label%3A%22good+first+issue%22

Currently, there are no Date pickers issues with that label, but we're looking into that.

@LukasTy LukasTy added component: pickers This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature and removed bug 🐛 Something doesn't work labels Feb 20, 2024
@LukasTy LukasTy changed the title [DateTimeRangePicker] Fix OK button behavior [DateTimeRangePicker] Change OK button behavior Mar 5, 2024
@LukasTy
Copy link
Member

LukasTy commented Mar 5, 2024

We decided to go with the suggested approach.
The first course of action would be to introduce a new action that would be shown, while the value is "incomplete" (all the specific cases TBD when working on it).
We discussed that it would make sense for this action to have a different label (maybe "next"?).

@arthurbalduini
Copy link
Member

arthurbalduini commented Sep 16, 2024

I had some insights and challenges while working on this issue. Perhaps it can be useful for future discussions about not only the action bar, but for all the slots we use in the Pickers:

Creating a new next action. Demo

  • Such an action only makes sense to be used on the start position of rangePickers
  • For better DX, it was necessary to add the rangePosition information to the actionBar ownerState in order to allow users to conditionally use the new action. The most clear use case is to be able to not render the Next action when selecting an end date.
  • Also regarding DX: for "non-range" pickers we do not want to suggest the existance of the rangePosition attribute on the owner state, and to do so the we need to have decoupled interfaces for those

Keep only the accept action (ok button) and change its behavior to move range position on range pickers

  • Our current actions are very straightforward to use, with a single behavior associated to each action. IMHO, providing a "dual" behavior for the accept action on RangePickers is a BC and step on a direction that, if we decide to go back in the future, would also be a BC
  • The current behavior would no longer be reproducible

Last consideration is that the ActionBar is defined on the community package, so adding range information requires passing additional props to common hooks, for example to change the range position, which doesn't improve the code readability. It also touches the Pickers Layout, which is also common among the packages, so the complexity scales in different layers.

@arthurbalduini
Copy link
Member

Similar to what was commented here, we will postpone this task for v8. We could use this time to define which solution suits better our needs. Some insights from @joserodolfofreitas and @flaviendelangle would be very appreciated 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: DateTimeRangePicker The React component component: pickers This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature v8.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants