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

Replace MUI DatePicker with react-datepicker #986

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

snoesberger
Copy link

This PR fixes #948 #920 #614 and, as mentioned in #614 (comment), the react-datepicker brings also some advantages over the MUI DatePicker.

With this PR, there is still the statistics component that uses the MUI DatePicker. I haven't been able to replace it because the stats feature doesn't work properly in the new admin UI, see #282. Once this is working again, the MUI DatePicker can also be replaced there and the dependencies can be cleaned up.

Fixes several issues with date picking.
react-datepicker allows date-range-picking and makes it therefore easier to handle date picking.
…diting

Fixes several issues with date picking.
react-datepicker allows date-range-picking and makes it therefore easier to handle date picking.
Copy link
Contributor

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-986

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-986

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

Copy link
Contributor

This pull request is deployed at test.admin-interface.opencast.org/986/2024-11-29_13-23-18/ .
It might take a few minutes for it to become available.

@snoesberger snoesberger requested a review from Arnei November 29, 2024 13:26
@snoesberger snoesberger added type:bug Something isn't working type:visual-clarity Improves UI readability labels Nov 29, 2024
@marcusm7
Copy link

marcusm7 commented Dec 5, 2024

This pull request is deployed at test.admin-interface.opencast.org/986/2024-11-29_13-23-18/ . It might take a few minutes for it to become available.

I looked into the above mentioned link. Selecting dates in the Startdate-filter, event-schedule and event-metadata works fine. Would it be possible to focus the datepicker when it is opened - having the focus allows navigation with i.e. cursor-keys and pgup/pgdown. Getting the focus manually needs some learning as the picker closes on single-click, tab closes the picker.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

This pull request is deployed at test.admin-interface.opencast.org/986/2024-12-13_16-06-54/ .
It might take a few minutes for it to become available.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-986

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-986

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

@marcusm7
Copy link

marcusm7 commented Dec 6, 2024

This pull request is deployed at test.admin-interface.opencast.org/986/2024-12-06_08-16-39/ . It might take a few minutes for it to become available.

Nice. :) Focus works for me.

$ plasmashell --version
plasmashell 6.2.3
$ firefox --full-version 
Mozilla Firefox 133.0 20241121140525 20241121140525

The date range picker for the start date filter no longer has a year (I'm sorry I didn't see it yesterday).
It's fine by me, I can just press and hold pgup or pgdown for a short while (or shift+pgup/down for year) to get to the year I want. But maybe there is an easy way to add a UI-selector for year or year-month?

In the event schedule start date: When I try to change the year (yyyy) by typing it in, the year is evaluated after the 2nd digit, I would have expected, that it lets me type all 4 digits - this is not related to the react-datepicker I think?

@snoesberger
Copy link
Author

The date range picker for the start date filter no longer has a year (I'm sorry I didn't see it yesterday). It's fine by me, I can just press and hold pgup or pgdown for a short while (or shift+pgup/down for year) to get to the year I want. But maybe there is an easy way to add a UI-selector for year or year-month?

Yes, I thought about this too, and there is even a way to add a drop down with month/year to choose from (showMonthYearDropdown prop). But the thing is, if you want to use this drop down, you also have to define a minDate and a maxDate. This will limit the dates you can choose from. (see also the following issue: Hacker0x01/react-datepicker#2553)
Because it's not easy to choose a min and max date that suits everyone, I didn't activate the drop-down. And as you mentioned, you can still quickly change the year with shift + pg up/down.

In the event schedule start date: When I try to change the year (yyyy) by typing it in, the year is evaluated after the 2nd digit, I would have expected, that it lets me type all 4 digits - this is not related to the react-datepicker I think?

This problem doesn't seem to be related to the react-datepicker, at least I have similar problems with the old datepicker. Maybe it has to do with conflict checking? We should probably cover this in another issue.

@snoesberger
Copy link
Author

The date range picker for the start date filter no longer has a year (I'm sorry I didn't see it yesterday). It's fine by me, I can just press and hold pgup or pgdown for a short while (or shift+pgup/down for year) to get to the year I want. But maybe there is an easy way to add a UI-selector for year or year-month?

Yes, I thought about this too, and there is even a way to add a drop down with month/year to choose from (showMonthYearDropdown prop). But the thing is, if you want to use this drop down, you also have to define a minDate and a maxDate. This will limit the dates you can choose from. (see also the following issue: Hacker0x01/react-datepicker#2553) Because it's not easy to choose a min and max date that suits everyone, I didn't activate the drop-down. And as you mentioned, you can still quickly change the year with shift + pg up/down.

I had another look at it and found another way to achieve the ability to easily change the month and year. I added the functionality in the last commit.

@marcusm7
Copy link

marcusm7 commented Dec 7, 2024

The date range picker for the start date filter no longer has a year (I'm sorry I didn't see it yesterday). It's fine by me, I can just press and hold pgup or pgdown for a short while (or shift+pgup/down for year) to get to the year I want. But maybe there is an easy way to add a UI-selector for year or year-month?
...
I had another look at it and found another way to achieve the ability to easily change the month and year. I added the functionality in the last commit.

https://test.admin-interface.opencast.org/986/2024-12-06_14-32-46/

I had a look into the the date range selection of the start date filter.
It now has dropdowns for month and year, which work as intended by you. 😀

image

$ plasmashell --version
plasmashell 6.2.3
$ firefox --full-version 
Mozilla Firefox 133.0 20241121140525 20241121140525

I'm not able to focus the dropdowns via tab-cycle, but I don't see it as a big issue since the (shift+) pgup/down-keys are there.

@marcusm7
Copy link

marcusm7 commented Dec 7, 2024

In the event schedule start date: When I try to change the year (yyyy) by typing it in, the year is evaluated after the 2nd digit, I would have expected, that it lets me type all 4 digits - this is not related to the react-datepicker I think?

This problem doesn't seem to be related to the react-datepicker, at least I have similar problems with the old datepicker. Maybe it has to do with conflict checking? We should probably cover this in another issue.

I added an issue: #1005

@Arnei
Copy link
Member

Arnei commented Dec 13, 2024

Minor styling issue. The "up/down" buttons for the year selector don't look the part:
Bildschirmfoto vom 2024-12-13 09-47-58

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Code looks good to me and works from what I can tell :) Only found the one minor styling issue above.

@snoesberger
Copy link
Author

Code looks good to me and works from what I can tell :) Only found the one minor styling issue above.

Thanks for the review! I addressed the styling issue you mentioned in my last commit. This is what it looks like now:
grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working type:visual-clarity Improves UI readability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filter, Start Date: Selecting same day start date isn't automatically recognized
3 participants