-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add rudimentary timeslot support #1087
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1087 +/- ##
==========================================
- Coverage 81.29% 80.84% -0.45%
==========================================
Files 141 142 +1
Lines 5089 5154 +65
==========================================
+ Hits 4137 4167 +30
- Misses 952 987 +35 ☔ View full report in Codecov by Sentry. |
Update timeslot (Create is almost identical, just missing the empty extra form at the bottom.) Note the dark background in the input fields. I suspect our "argus"-theme cannot be used together with dark mode, there's something we've forgotten to set. This should not be fixed per input-field but globally, once. |
7dddb7e
to
acfd5c5
Compare
Quality Gate passedIssues Measures |
New screenshot of the update/create page after we found the right way to fix the input field backgrounds in #1089 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an absolute bare minimum timeslots page, with potentially some bugs (see comments below) and much follow-ups needed. The page is borderline un-understandable because of the UI (when I click on "Update" button somewhere in the middle, I am not immediately sure whether I will update the timeslot above or below f.e.), so some basic daisy-"cardification" plus minimal page organization with basic flexbox setup is high priority next step I would say. I suggest we make a meta issue and include this PR as one of the tasks there.
Do I understand correctly that some views seem redundant but are probably just a transitional step before we land on the page design (see comments below)? Also return values of most views will need to be changed in the future. F.e. we have redirects on timeslot delete action, while we need just to show the confirmation modal for it.
After a manual test this PR seems OK to me functionality wise (except what's mentioned in the comments below). Timeslots and recurrences are updated/created as expected (although the functionality is difficult to navigate in).
There will be HTML updates needed (adding h1, wrapping with fieldsets), but those belong to separate PRs.
See #1094 |
It works but is almost indescribably fugly. No magic, to add another time recurrence it is necessary to "update" more than once.
As we plan to entirely hide the implementation of time recurrences, this is fine.