Skip to content

Conversation

@Chartman123
Copy link
Collaborator

@Chartman123 Chartman123 commented Apr 19, 2025

This is a follow-up on #2646 to support restrictions and range for time questions.

Set question settings for minimum/maximum time values and if the user must enter a time range:
image

Restricted times and time range picker in submit view:
grafik

Result in summary / responses view:
grafik
grafik

@codecov
Copy link

codecov bot commented Apr 19, 2025

Codecov Report

Attention: Patch coverage is 75.86207% with 7 lines in your changes missing coverage. Please review.

Project coverage is 43.93%. Comparing base (d1975dc) to head (c040863).
Report is 9 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2712      +/-   ##
============================================
+ Coverage     43.72%   43.93%   +0.20%     
- Complexity      921      941      +20     
============================================
  Files            78       78              
  Lines          3403     3428      +25     
============================================
+ Hits           1488     1506      +18     
- Misses         1915     1922       +7     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Chartman123 Chartman123 force-pushed the feat/add-time-range branch 7 times, most recently from 6b264a2 to 97ba57a Compare April 19, 2025 12:20
@Chartman123 Chartman123 self-assigned this Apr 19, 2025
@Chartman123 Chartman123 added enhancement New feature or request php PHP related ticket javascript Javascript related ticket 3. to review Waiting for reviews feature: ❓ question types labels Apr 19, 2025
@Chartman123 Chartman123 added this to the 5.2 milestone Apr 19, 2025
@Chartman123 Chartman123 marked this pull request as ready for review April 19, 2025 12:25
@Chartman123 Chartman123 requested a review from susnux April 19, 2025 12:26
@Chartman123 Chartman123 force-pushed the feat/add-time-range branch 2 times, most recently from 0259ee4 to a7a7c77 Compare April 19, 2025 13:22
@Chartman123
Copy link
Collaborator Author

@susnux the NcDateTimePickerNative doesn't show the icon on the right to open the picker anymore. Is this a bug in the vue lib? But it did work when I started working on this... So not sure... 😉

@nimishavijay what do you think about the icons for the actions? I'm not really happy with them, but didn't find better ones...

@nimishavijay
Copy link
Member

Regarding the icons, I agree it's a tricky one. What do you think of this clock loader 20 and clock loader 80?

And IIRC we are more and more adopting the native time picker by the browser and not the time picker component in the screenshot, which we previously had (for example, in the Calendar app we stopped using the time picker component). I would suggest using 2 native time picker fields with a "to" or "-" in the middle to indicate range and the allowed times in a subline so that it looks up-to-date :)

@Chartman123
Copy link
Collaborator Author

@nimishavijay regarding the picker type: the NcDateTimePicker will stay around for ranges, that's at least what I found in the docs. So I'd like to stick with it as long as this is still possible. :)

What do you think of this clock loader 20 and clock loader 80?

They would have been my second choice, too

@nimishavijay
Copy link
Member

regarding the picker type: the NcDateTimePicker will stay around for ranges, that's at least what I found in the docs. So I'd like to stick with it as long as this is still possible. :)

No problem! In that case I'd suggest thinking about how we can differentiate the start and end times more, as now it is just one field and seemingly one large picker. Also FYI it looks pretty different in the style guide (I'm not sure when this change happened, maybe @susnux knows more)

@Chartman123
Copy link
Collaborator Author

No problem! In that case I'd suggest thinking about how we can differentiate the start and end times more, as now it is just one field and seemingly one large picker. Also FYI it looks pretty different in the style guide (I'm not sure when this change happened, maybe @susnux knows more)

Yes, I think the style guide is already based on v9 if you don't specify v8 explicitely :)

@Chartman123 Chartman123 force-pushed the feat/add-time-range branch from a7a7c77 to 9389f81 Compare April 25, 2025 20:14
@susnux
Copy link
Collaborator

susnux commented Apr 28, 2025

NcDateTimePickerNative doesn't show the icon on the right to open the picker anymore

This is browser dependent as this is the native browser input and cannot be controlled by us.

@Chartman123 Chartman123 force-pushed the feat/add-time-range branch from 9389f81 to e83096a Compare April 28, 2025 08:08
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
@Chartman123 Chartman123 force-pushed the feat/add-time-range branch from e83096a to c040863 Compare April 28, 2025 08:15
@Chartman123
Copy link
Collaborator Author

@nimishavijay I've changed the icons now and I think that they look much better. 👍🏻 (screenshot updated in OP)

Copy link
Collaborator

@susnux susnux left a comment

Choose a reason for hiding this comment

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

codewise it seems to be fine ✅

@Chartman123
Copy link
Collaborator Author

@nimishavijay There's now a new feature request about switching the date/time pickers to the native ones in #2719. So we'll change that for all date/time related questions in a separate PR 👍🏻

@Chartman123 Chartman123 merged commit 32e1fec into main Apr 29, 2025
54 checks passed
@Chartman123 Chartman123 deleted the feat/add-time-range branch April 29, 2025 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement New feature or request feature: ❓ question types javascript Javascript related ticket php PHP related ticket

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants