Bug 1731987
| Summary: | Make New compliance policy page consistent with rest of satellite UI. | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Satellite | Reporter: | Jameer Pathan <jpathan> | ||||
| Component: | SCAP Plugin | Assignee: | Aditi Puntambekar <apuntamb> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Jameer Pathan <jpathan> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | 6.6.0 | CC: | egolov, ltran, mhulan, mmccune, mshriver, mzalewsk, oprazak | ||||
| Target Milestone: | 6.10.0 | Keywords: | Triaged | ||||
| Target Release: | Unused | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | tfm-rubygem-foreman_openscap-4.2.0 | Doc Type: | If docs needed, set a value | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2021-11-16 14:08:51 UTC | Type: | Bug | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Attachments: |
|
||||||
|
Description
Jameer Pathan
2019-07-22 13:36:18 UTC
I took another look at this page and our code and I believe the most important part here is that usually <input> is children of <label>, while on this page <label> is uncle to <input>. This requires us to add special handling just for this one page, which is good indicator of possible inconsistency in Satellite. That prompted this bugzilla as first approach on fixing the root cause. We don't really mind if radio boxes are displayed in one line, or one below another. If there is another page in Satellite with the same radio buttons group HTML structure, please let me know. Might be we are missing support for that and it's really on us to provide it. Created redmine issue https://projects.theforeman.org/issues/27378 from this bug Mirosław, thanks for more detailed description. Though the observation is not entirely correct. We have label for the form field and label for radio button itself. E.g. go to subnet page and see the protocol selection. The label protocol is the main one, each radio has own label. The main one is a sibling, while each radio button has more specific child label. The same is on other forms, e.g. job invocation form. In this deployment step, there are three main fields which are connected. It wouldn't look great as inline field with three options, also the inline help would be impossible to split per each value. Is this a big issue for automation? If that helps, we can add unique ids to all inputs or their labels. Marek, Relevant part of our automation is here: https://github.com/SatelliteQE/airgun/blob/master/airgun/widgets.py#L66 This code assumes that <input> element is descendant of <label> element (the one that provides text for this specific radio box, not field in general). Such assumption is true for subnets. Main problem is associating specific radio box with human-readable label. Looking at https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label , it seems there are two main ways to do this: 1. Make <input> direct child of <label> - this is what Satellite usually does and what our automation assumes 2. Add "id" attribute to <input>, with matching "for" attribute in <label> My proposal is: 1. Add "id" to <input> and "for" in <label> on discussed page on Satellite side (this will have added benefit of allowing users to click on labels to select radio box - we only need to ensure clicking on info icon does not trigger that action) 2. On QE side, teach automation to work with <labels> associated with <inputs> through id and for attributes. It will also make it a little easier for us in QE if we group all radio boxes in one <div> (see attached screenshot for visualization). It doesn't need to impact page look in any way, it's only for us to ensure we don't include too many elements by mistake. Hope it sounds reasonable to you Created attachment 1594878 [details]
multiline radio
Totally makes sense and should be easy to add. I'm a bit lost on this. Are we asking UX team to correct the page to make it consistent with the rest of satellite or are we changing our automation to fit this current page? If the former, has this been put in UX's sprint to work on yet? My understading is, we are adding ids to labels, not changing the way it looks. See comment 5 for why I don't think we should change the layout. I suppose the automation may need some changes after ids are added. Agreed with Marek. In my mind, it was always about shuffling HTML structure and attributes around, with little or no impact on actual look of the page. Upstream bug assigned to apuntamb Upon review of our valid but aging backlog the Satellite Team has concluded that this Bugzilla does not meet the criteria for a resolution in the near term, and are planning to close in approximately a month. If you have any concerns about this, please contact your Red Hat Account team. Thank you. I've sent a patch implementing the suggestion 1 from the comment 6. Labels are clickable now, little help icons remain to display only the help. That should resolve this BZ. Moving this bug to POST for triage into Satellite since the upstream issue https://projects.theforeman.org/issues/27378 has been resolved. Verified: Verified with: - Satellite 6.10.0 snap 12 - tfm-rubygem-foreman_openscap-4.3.3-1.el7sat.noarch Test step: - Go to Host > Policies > New compliance policy - Inspect page Observation: - "id" is present in <input> tag and "for" in <label> tag. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (Moderate: Satellite 6.10 Release), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2021:4702 The needinfo request[s] on this closed bug have been removed as they have been unresolved for 500 days |