confusing date/time form helper in reboot dialog
Garrett │ pitti: looks mostly fine. Invalid date format warning should be under the date, not time.
This isn't just a bug, more of a mis-design. If both are invalid, we show it like this:
Presumably to avoid the strings becoming too long and not fitting next to each other?
So let's do this as a follow-up.
Originally posted by @martinpitt in https://github.com/cockpit-project/cockpit/issues/20668#issuecomment-2192189042
@garrett How about we shorten the string to "invalid" and handle time and date separately? Then the string is hopefully short enough (translations!) to fit into the field width.
PatternFly has the (!) and error states for inputs when things are invalid:
https://www.patternfly.org/components/forms/form#invalid
Can we set the states for the invalid forms properly?
Also, the elements are overlapping each other for some odd reason. Crop to highlight:
Specifically, the date and time should look like this:
I'm not sure why the (!) is on opposite sides. I guess the calendar is a button and the clock is an icon. And the text is about invalid versus asking for a valid time. It's weird and inconsistent.
We could have 3 strings (which could all be translated):
- Invalid date
- Invalid time
- Invalid date and time
And then have the corresponding field(s) in error mode.
And date should always be preselected as current date, right? (It doesn't look like this in the screenshot — but why not?) So the errors will mainly be the time, not date. I guess we could even prefill the time as 5 minutes (assuming someone would want at least a few minutes if they're not picking immediate) in the future as a default, which could be changed.
Mockups. First is the label on the left size (without wrapping), but this is a little too wide. We might actually want to use stacked labels (which is the PF default, but we usually do not use... although switching it up in some places and considering it for the future where we can might not be a bad idea).
The other things that are in common between the two layouts:
- Short strings for either date or time
- Strings are directly below each widget
- Date and time widgets use their PF error states
- Date is set to current date (in 5 minutes from the current time, so the time is in the future)
- Time is set to 5 minutes in the future
- 5 minutes is arbitrary, but I wanted to select something in the future, as if they wanted to reboot immediately, they would have done so. Having a good default is better than not suggesting anything. It could be 10 minutes, but I didn't want to guess too far ahead.
- Setting the time to the past should bump the date to the next day (if in the past), as you cannot reboot the machine in the past (it's not a time machine).
I just realized I didn't add the timezone. The most obvious thing to do would be to show the Cockpit system's timezone right after the time.
Additionally, perhaps the helper text for "computer will reboot in X minutes" would be for the modal, instead of below the time?
If so, it'd look like this:
(Error states in the designs above would still apply.)
OK, here's one where I backported the changes from the default into the merged set of mockups:
Anyway, I think this should be handled in multiple PRs; I don't expect it to be handled all at once. (This is part of the reason why I'm suggesting the stacked version and have the side version mocked up still.)
For a point of reference, I think it's good to have a short string that's right under the error. Here's what it looks like without being aligned (either to the group datetime group or the whole group):
If anything, the second of these would be preferable, I think... but the real ideal one would be directly under the input.
But if we do default to a suggested date and time, then that removes the chance that someone will end up in an error state.