cockpit-podman
cockpit-podman copied to clipboard
Fix deletion confirmation modal

Issues:
- Title is truncated, with no way to see what it is.
- This can be always set to wrap by changing the
white-space: nowrap;towhite-space: break-spaces; line-clampcould help here by letting it be a specified number of lines before truncating (instead of just 1), but it's still under a-webkit-prefix and requires a bunch of wacky other bits of CSS to have it work. However, it does work cross browser in a reliable, supported manner. https://css-tricks.com/almanac/properties/l/line-clamp/ (This is incompatible with the above suggestion, which is simpler and fine as long as the line doesn't run too long.)
- This can be always set to wrap by changing the
- Modal is missing content to explain what it will do and to what it will do it to.
- There's no need to have "Confirm ..." in the modal. That's redundant.
- https://www.patternfly.org/v4/components/modal/design-guidelines/#confirmation-dialogs
- I thought there were guidelines for writing modal confirmations, but I couldn't (quickly) find them. You can see in the demo that there shouldn't be a redundant title. The entire point of the modal is to ask for confirmation. I guess https://www.patternfly.org/v4/ux-writing/ux-writing-best-practices#use-positive-action-oriented-language kind of covers it in a way? (Especially in the "Be clear and concise" section.)
Originally posted by @garrett in https://github.com/cockpit-project/cockpit-podman/issues/1037#issuecomment-1246748355
This deletion modal is from deletion a pod, but we should also check other dialogs.
This deletion modal is from deletion a pod
Yeah, I wasn't sure, as it was truncated and didn't provide any context in the screenshot... :sweat_smile: If some of us on the team are having issues identifying things, then you can be sure our users won't know. :wink:
Agreed, we should verify all of them and see if they suffer from the same issues.
I've worked on 2. and 3., for 1. I am not sure what we want to do.
break-spaces doesn't help a lot:

Ideally patternfly would apply this on mobile right? Should we fill an issue?
https://github.com/patternfly/patternfly/issues/1825 is an old issue about this topic. Specifically this comment states it was "recently" worked on in 2020 https://github.com/patternfly/patternfly/issues/1825#issuecomment-638405853:
Just revisiting this. In a recent PR, we added support for truncation via css line-clamp which allows you to specify the number of lines shown before truncation happens (eg, truncate after 2 or 3 lines of text). Maybe we could support that feature on the modal title, too? And potentially an opt-in for setting no truncation?
I do see line-clamp (as I suggested in this issue) as a helper utility in PF @ https://github.com/patternfly/patternfly/search?q=clamp — but it's only used for expandable sections, drawers, and alerts at the moment.
Having the image tag in the title seems redundant when it's also in the body. And in the below situation it's even confusing. The title says "test-busybox:latest" will be deleted even though "test-busybox:4" is selected in the body.
This is how it should look according to PatternFly guidelines:
- Modal confirmation of destructive action: https://www.patternfly.org/components/modal/design-guidelines#confirm-a-destructive-action
- Indeterminate all, nested list of checkboxes: https://www.patternfly.org/components/forms/checkbox/design-guidelines
Additionally:
- I removed the prefix and suffix for the image name in the title, to conserve space.
- I let the title wrap (instead of truncate) so it's clearer what is going on in mobile. (With the shortened name, it will fit better.)
- I reworded the text.
Notes: All would be selected by default. Mixed selections would have the "All" selection turn indeterminate. Everything unselected would make "All" unselected.