feature(web): make the product selection more appealing and accessible

[lkocman]

• Fixes Icons for individual distros #1415
• Supply icon for all products and copy them to dist/assets/logos in final web build.
• Add icon to structures, pass it to dbus, etc (please do not mix product icon with pattern icon) fallback to default.svg in case that no icon is set

Before	After

---

For more details please read #1550 (review) (and maybe #1550 (review))

[jreidinger]

I expect this padding in CSS, but sadly I am not css expert. Lets hope @dgdavid gets back soon

[dgdavid]

This is not a change from @lkocman. In any case, it is just using available props of a PatternFly component. They are later translated in style changes. I tend to avoid using layout props as much as possible, but these ones are for a responsive layout. Let's see if we manage to find a better way in the future, but for now it's the way to go in this special cases.

[jreidinger]

same here. I expect icon style to be in css

[dgdavid]

same here. I expect icon style to be in css

Exactly, styles should live in a CSS file instead of be hardcoded in the component. Would be nice to make something generic in order to Be the browser’s mentor, not its micromanager. At this moment Agama CSS files are a bit messy, since we still a few more cleanup rounds to get them in a better shape.

I can help @lkocman with this since I fully understand it is a bit painful to deal with this topic due to the situation. In the future, almost all of this things should be themeable.

[jreidinger]

also here I expect some class and style defined in css.

[dgdavid]

also here I expect some class and style defined in css.

Sure, let's extract these things to CSS rules but carefully for not ending having thousand of rules for every thing. I'll come up with some proposals on top of this once I find a bit of time to address it.

But have in mind two things,

• Here @lkocman is forcing a cursor because using the onClick prop for the card after dropping the radio button. It's not wrong, but I'd prefer to somehow revert the behavior for laying out the card content with the radio and its label if possible. The benefit it that the browser will be able to hint the users without the need of changing the styles. Will be clear with an example (to be created)
• The fact that border is styled as none when the current product is selected plus the "Change" button disabled and a tiny bug I found in master branch with the focus lost because a re-draw makes me think that currently selected product shouldn't be part of the options to select. Our fault, but something to think about and, most probably, to improve.

[jreidinger]

@dgdavid after consulting with patternfly documentation, I would say that for product selection we maybe can use Tile component, what do you think? https://www.patternfly.org/components/tile#with-long-subtext

[lkocman]

@dgdavid oky then

[dgdavid]

BTW, manually testing your changes I've found an a11y bug on the master branch: the focus is lost because the entire selector is redrawn every time the selection changes. A bit technical, but this happens because it is a controlled form in order of being able to know if the selection matches the already selected product. This, along with a condition you used for adding a border or not depending on the same condition, made me think that it does not make sense to have the selected product as part of the selector. Of course, it should be on the selection page, but not as an option, although this is for another PR 😉

As said, not related to this PR but I took the opportunity and fixed it as part of it :) See 3cb7ebf

It wasn't a (direct) consequence of being a controlled form but because a nested component definition. Anyway, users can now select the product using the keyboard too, but still thinking that selected product should not be part of the offered options. A better layout is needed to inform the user not only about the currently selected product but also about the consequences of changing it... but, now for real, in another PR 😃

[dgdavid]

@dgdavid after consulting with patternfly documentation, I would say that for product selection we maybe can use Tile component, what do you think? https://www.patternfly.org/components/tile#with-long-subtext

Sorry @jreidinger, I had overlooked this comment.

No please, do not use PF/Tile. It will be deprecated in favor of PF/Card, see patternfly/patternfly-react#8542. I mentioned it a couple of months ago when working in storage part, see 8d432fb, which is also no longer avialable.

The idea, as commented in team meetings, is to keep component as simple as possible, closer to HTML the better. At least by now, since there a lot of things to do in a limited period of time :) In any case, the whole UI will be benefit from using simple, yet understandable elements or components.

Thanks a lot for having a look anyway. You rocks!

[dgdavid]

@lkocman, @jreidinger,

Here I go 🙄

I've reworked the proposed changes for having these logos in the UI but keeping it as much accessible as possible with our (yet limited) knowledge to that regard.

But before entering in details, let me thanks @lkocman again for the contribution since it has been not only an step forward for having the product logos in the selection page, but hopefully a turning point regarding to a11y in Agama.

As you can see in the changes, I've removed the onClick prop from the PF/Card. If you go commit by commit, you'll see that I restored it at some point, but a11y Firefox tool complained about https://developer.mozilla.org/en-US/docs/Web/Accessibility/Understanding_WCAG/Keyboard#clickable_elements_must_be_focusable_and_should_have_interactive_semantics.

Thus, I've used a CSS technique for expanding the area of the label to the whole card in addition to force the pointer cursor for it in this case (by default, labels are clickable but use the default cursor). That way, what @lkocman was looking for is achievable without defeating the keyboard usability (I believe).

You may wonder why I didn't use the PF/Card with selectable and clickable properties. I simply did not have as much PF knowledge as I'd like and also find selectable&clickable card rather complex at this moment.

I also tried to follow and accomplish the PF/Card a11y testing check list, https://www.patternfly.org/components/card/accessibility#testing

Since product descriptions are usually long, I have marked them as aria-details in order to prevent the screen reader reading the whole text before announcing the "not selected radio button" or "selected radio button" state, which would be a bit annoying IMHO. What is more, from my POV these descriptions perfectly fits as details.

Last but not least, you will see that I have added the "Select a product" label (actually a legend) there. I know it's tempting to remove it, but users need a context. Not you, not me, not even most of the guys we know... but there are users that need such a context. I'm saying that because it's true that an aria attribute will be enough for screen reader users, but I really believe that such a selector needs a bit of context when a user lands there. In other words, IMHO it is not superfluous but contributes.

You can check the visual result by yourself. And play the below videos to see how keyboard and screen reader are supported.

• Using the selector with mouse and keyboard

  product-selection-keyboard-support.mp4

• Using an screen reader and keyboard (you might need to unmute the video)

  product-selection-screenreader-support.mp4

Please, look at this as just an step forward. Any other change or improvement like

A better layout is needed to inform the user not only about the currently selected product but also about the consequences of changing it

can be properly addressed in a next iteration in order to not end up with an endless PR.

Thanks a lot!