Implement read-only view of individual collection page#3636
Conversation
|
Skipping CI for Draft Pull Request. |
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
0905fb8 to
bf65ad2
Compare
| } | ||
|
|
||
| /* Un-override the above override when the <Select> component is disabled */ | ||
| .pf-c-select__toggle.pf-m-disabled::before { |
There was a problem hiding this comment.
Before this change:
After this change:
Disabled <Select> styles on the PatternFly site:
There was a problem hiding this comment.
Also note the subtle color difference between the top and bottom dropdowns. This is due to an opacity: 0.5; rule from the ui-components package that is applying to <button> elements. The top <Select> uses a button as the main element while the bottom uses a <div> due to the difference PF options in use.
| <Title headingLevel="h4"> | ||
| There are no other collections attached to this collection | ||
| </Title> | ||
| </EmptyState> |
There was a problem hiding this comment.
This is consistent with many other empty state titles. However, a question for PatternFly Developer Office Hours is pro and con for accessibility of h4 element versus p element with the identical style.
In a video for a past day of learning with theme of accessibility, the screen reader user used heading structure of page quite a bit to form a mental model of the information hierarchy (and commented on any skips in heading levels). This page has intuitive hierarchy for h1 and h2 but not so much for this heading.
There was a problem hiding this comment.
True I agree, especially with the screen reader comment since this bit of text doesn't really communicate hierarchy. The page has h1 and h2, but there isn't anything semantically that makes this a h3, h4, etc. I'll update to a <p> with styles.
| {isReadOnly ? ( | ||
| <> | ||
| <Title className="pf-u-mb-xs" headingLevel="h2"> | ||
| Attached collections |
There was a problem hiding this comment.
Bravo! Does Add new collection rules deserve similar treatment?
There was a problem hiding this comment.
On the other side of the coin, consistent Attached collections heading is better for docs team seems clearer (to me, at least).
There was a problem hiding this comment.
Good point. How about "Attached collections" in both modes, but conditional rendering of "Extend this collection by attaching other sets." which only applies in the edit modes? I think that would be a good call to action in combination with the "Attach" and "Detach" buttons below.
I'll take a look at the collection rules text too.
|
Images are ready for the commit at a8ca9d0. To use with deploy scripts, first |
pedrottimark
left a comment
There was a problem hiding this comment.
How did we ever survive without conditional rendering via React?
2 comments for your consideration.
ee2cea2 to
a8ca9d0
Compare
Description
This implements the read-only "view" mode of individual collection pages.
Form controls will be disabled and non-interactive, extraneous UI components (trash buttons) will not be visible, and the collection attacher component will be replaced with a read-only alternative display.
Checklist
If any of these don't apply, please comment below.
Testing Performed
Visit the collection page and select a collection from the table. The browser will navigate to the read only view for that collection with the constraints in the above description.
Clicking on disabled UI controls will have no effect. Tab-navigation will skip over the disabled controls.
When viewing a collection with no embedded collections in read-only mode, an empty state component will appear in place of the table:
Mobile layout views:
