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

Bug - DataList - No way to mix expandable and non-expandable content #7950

Closed
ianw opened this issue Aug 25, 2022 · 8 comments · Fixed by #8222
Closed

Bug - DataList - No way to mix expandable and non-expandable content #7950

ianw opened this issue Aug 25, 2022 · 8 comments · Fixed by #8222
Assignees
Labels
enhancement 🚀 good first issue PF4 React issues for the PF4 core effort
Milestone

Comments

@ianw
Copy link

ianw commented Aug 25, 2022

Describe the problem
I'm trying to replace some PF3 era ListView things with DataList. One thing that I can't seem to work around is that when mixing expandable and non-expandable rows, there's no way to keep the offset of the text consistent. Screenshots below are the easiest way to see this.

Some way of putting a blank DataListToggle would help. Or maybe it could have a "disabled" flag or something. Or is there some way to make this line up I'm missing?

Screenshots
Screenshot 2022-08-26 at 08-04-55 Zuul Build

Old view

Screenshot 2022-08-26 at 08-09-35 Zuul Build

@ianw
Copy link
Author

ianw commented Aug 26, 2022

I found that a "dummy" of

<DataListCell key='padding-icon' isIcon={true}>
  <span style={{paddingRight: "4em"}}></span>
</DataListCell>

on those rows that don't expand seems to push it out to the right size

Screenshot 2022-08-26 at 11-14-44 Zuul Build

@ianw
Copy link
Author

ianw commented Aug 30, 2022

Just to follow up on #7950 ... this is clearly not a good way to pad it out, as it seems to change based on something I can't quite figure out in different situations (between local testing environment and the site deployed by our CI environment, see below)

839222ad55bcbaeaf3bcb3d27bb2d888 nCWmXdmR8IfVs3QXpr9dXWUJdr1LcGZA

@ianw
Copy link
Author

ianw commented Aug 30, 2022

My next attempt is with a fixed-size span to simulate the icon, and using the item-control margin on it; this seems to work

<DataListCell key='padding-icon' isIcon={true} style={{marginRight: 'var(--pf-c-data-list__item-control--MarginRight)'}}>
  <span style={{display: 'inline-block', minWidth: '40px'}}></span>
</DataListCell>

@mcoker
Copy link
Contributor

mcoker commented Sep 3, 2022

Hey @ianw, the datalist component does not support columnar layout in all cases. Assuming it's a good fit, typically we recommend the table component if you are trying to maintain column alignment. Would that work?

--

Some way of putting a blank DataListToggle would help. Or maybe it could have a "disabled" flag or something.

FWIW, just some thoughts while thinking through this, this would probably be the easiest way to support that now. I created a demo here to show a few of ways of doing that - https://codepen.io/mcoker/pen/PoeoBjE. The solution is basically to use CSS to hide the data list control element visually, but retain its layout in the page, and make sure the element and any children (the toggle button) can't be interacted with and are not read by any assistive technology.

  • The first method uses aria-hidden and "opacity: 0;" on the data list control element to hide it and any of its children, disabled on the button to remove any interactivity with it. This method should work reliably.
  • The second method uses visibility: hidden; in CSS on the toggle directly. Technically this should work since visibility: hidden will remove an element from the tab order and keep it from receiving any events, but I haven't tested this outside of VoiceOver (works fine there) so it might be worth testing.
  • The third method uses inert along with opacity: 0 on the control element. It's worth noting that inert is a relatively new feature, and may not work or work consistently in all browsers. Here are the inert MDN docs and inert on caniuse to see browser support.

Do you want to give any of those a shot? We could look at adding this support to the component, too.

And another solution on our end could be to refactor the way we create the layout for the data list to allow for this, without the need to create hidden elements in the layout for alignment purposes. We support the combination of expandable and non-expandable nodes in some of our newer components, like the tree view component, so we could use a similar layout in data list though that update may be the kind of thing we consider in a major release as it might be breaking.

@ianw
Copy link
Author

ianw commented Sep 5, 2022

Hey @ianw, the datalist component does not support columnar layout in all cases. Assuming it's a good fit, typically we recommend the table component if you are trying to maintain column alignment. Would that work?

Yeah, I was trying not to rewrite it too much :) I would say the datalist works well for this; except for this small issue.

* The first method uses `aria-hidden` and "opacity: 0;" on the data list control element to hide it and any of its children, `disabled` on the button to remove any interactivity with it. This method should work reliably.

I can set the opacity in our CSS and that works to hide it, but I couldn't figure out to disable the button using React? I just have have a <DataListToggle> element which then puts the button in?

Do you want to give any of those a shot? We could look at adding this support to the component, too.

I obviously don't have a great global view, but something like a <DataListToggle disabled={true} /> that just put this blank element in would be easy for me to use :)

For reference the upstream change that I'm working on is https://review.opendev.org/c/zuul/zuul/+/854556. We have good CI on this but it does take a couple of clicks if you're looking at that review in gerrit; you'd go

"Zuul Summary" -> zuul-build-dashboard-opendev -> (shows the CI run) -> Artifacts tab -> Site Preview -> Click on "Builds" column of OpenStack -> choose any job to examine -> click "Console" tab.

A follow-on change https://review.opendev.org/c/zuul/zuul/+/855297 fiddles the CSS to make the datalist more compressed, which was asked for in reviews there. That's neither here or there, but it is more what we want to commit.

@mcoker mcoker transferred this issue from patternfly/patternfly Sep 7, 2022
@mcoker
Copy link
Contributor

mcoker commented Sep 7, 2022

Per the discussion above, I think the easiest way forward would be to add a prop to <DataListToggle> that allows props to be spread to the <Button> inside of it. Adding disabled aria-hidden="true" would disable the button in the way we want, and using CSS opacity or visibility to hide it visually would do the trick.

@mcarrano
Copy link
Member

@ianw will this address your problem? ^^^

@ianw
Copy link
Author

ianw commented Sep 13, 2022

@mcarrano -- yes; passing via props+some css works too :) whatever is most idiomatic for PF

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 good first issue PF4 React issues for the PF4 core effort
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants