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

Switch to native date picker #4668

Merged
merged 3 commits into from
Oct 18, 2023
Merged

Switch to native date picker #4668

merged 3 commits into from
Oct 18, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented May 11, 2023

Summary

Move to native datetime picker component as the old one has quite some issues especially with accessibility.

Shortcuts are now adapted to match the implementation of the reminders feature in files/talk.

No due date set Due date set
Screenshot 2023-08-22 at 17 18 23 Screenshot 2023-08-22 at 17 18 33
  • Cypress tests once UI is approved

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@cypress
Copy link

cypress bot commented May 11, 2023

1 failed test on run #1435 ↗︎

1 10 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 5ae067f160f434679902c2c0c4669c0b91b840ba into 80eddee...
Project: Deck Commit: 76a56df430 ℹ️
Status: Failed Duration: 02:51 💡
Started: Sep 12, 2023 2:06 PM Ended: Sep 12, 2023 2:09 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@juliusknorr juliusknorr force-pushed the enh/native-datepicker branch 2 times, most recently from 1e4e968 to 48d8f1c Compare August 22, 2023 15:20
@juliusknorr
Copy link
Member Author

@nextcloud/designers for visual review, screenshots are in the description 🙏

@juliusknorr juliusknorr changed the title enh/native datepicker Switch to native date picker Aug 22, 2023
@juliusknorr juliusknorr marked this pull request as ready for review August 22, 2023 15:25
@nimishavijay
Copy link
Member

Super nice! Only some small changes:

  • Use radio buttons for the icon (in files reminders the reminder can't be changed once set, so radio buttons could not be used, but it makes sense here)
  • Add a short indication of the date for the "This weekend" and "Next week" items. Eg. "This weekend - Sat, Aug 26, 8:00 AM"

@juliusknorr
Copy link
Member Author

Use radio buttons for the icon (in files reminders the reminder can't be changed once set, so radio buttons could not be used, but it makes sense here)

I'm not sure about that. The radio button only be selected as long as the day hasn't changed or the end of the day hasn't been reached. I feel without a radio button it is more logical as a single time action to set the date accordingly.

Add a short indication of the date for the "This weekend" and "Next week" items. Eg. "This weekend - Sat, Aug 26, 8:00 AM"

I like the idea, unfortunately there seems to be no way to properly localize the short form in terms that the order might be different (in Germany it would be 26. Aug for example) If accepting that we always have the same order no matter of which locale to use, then we could do that, otherwise I'd rather leave it out.

@juliusknorr juliusknorr force-pushed the enh/native-datepicker branch 4 times, most recently from 9bde832 to ce0e8e9 Compare October 18, 2023 12:39
Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Looks good. Just one minor suggestion.

src/components/card/DueDateSelector.vue Outdated Show resolved Hide resolved
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr juliusknorr merged commit 7df062d into main Oct 18, 2023
13 checks passed
@juliusknorr juliusknorr deleted the enh/native-datepicker branch October 18, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants