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

confusing date/time form helper in reboot dialog #20669

Open
martinpitt opened this issue Jun 26, 2024 · 6 comments
Open

confusing date/time form helper in reboot dialog #20669

martinpitt opened this issue Jun 26, 2024 · 6 comments

Comments

@martinpitt
Copy link
Member

          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:

image

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 #20668 (comment)

@martinpitt
Copy link
Member Author

@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.

@martinpitt martinpitt added question Further information is requested needsdesign and removed question Further information is requested labels Jun 26, 2024
@garrett
Copy link
Member

garrett commented Jul 8, 2024

PatternFly has the (!) and error states for inputs when things are invalid:

https://www.patternfly.org/components/forms/form#invalid

image

Can we set the states for the invalid forms properly?

Also, the elements are overlapping each other for some odd reason. Crop to highlight:

image

@garrett
Copy link
Member

garrett commented Jul 8, 2024

Specifically, the date and time should look like this:

image

image

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.

@garrett
Copy link
Member

garrett commented Jul 8, 2024

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).

image

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:

image

(Error states in the designs above would still apply.)

@garrett
Copy link
Member

garrett commented Jul 8, 2024

OK, here's one where I backported the changes from the default into the merged set of mockups:

image

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.)

@garrett
Copy link
Member

garrett commented Jul 8, 2024

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):

image

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants