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

EmptyTableCellForAlignment - migrate from ODH and document API #14

Closed
nicolethoen opened this issue Oct 8, 2024 · 2 comments
Closed
Assignees
Milestone

Comments

@nicolethoen
Copy link
Contributor

The code for this component currently lives https://github.com/opendatahub-io/odh-dashboard/blob/main/frontend/src/pages/projects/components/EmptyTableCellForAlignment.tsx

This new components should be built using PF6.

A PF6 version of this component lives in the current PF6 migration POC PR: opendatahub-io/odh-dashboard#3038

The API of the new component should match the API of the component in RHOAI.
We can improve a11y, right-to-left language support, replace deprecated components under the hood, or generally clean up the implementation, as long as the API works in RHOAI.

@github-project-automation github-project-automation bot moved this to Needs triage in PatternFly Issues Oct 8, 2024
@nicolethoen nicolethoen added this to the 2024.Q4 milestone Oct 8, 2024
@nicolethoen nicolethoen moved this from Needs triage to Ready to assign in PatternFly Issues Oct 8, 2024
@adamviktora adamviktora self-assigned this Oct 9, 2024
@adamviktora
Copy link
Collaborator

adamviktora commented Oct 21, 2024

This component is very small and I don't see a use case for making it a general component.

If I understand the purpose of it, it should be used as a gap filler when using the expandable table row. But as I was taking a closer look at the usage in the odh-dashboard, I am not sure if that still is the case. The hardcoded 46px is also concerning. If it was used only as the gap filler for the expanded row for the place of the toggle, simple Td will do the job and there is no need to hardcode the 46 (maybe it is a leftover from PF4).

Image Image

What might be an issue when migrating to V6 is the PatternFly ExpandableRowContent component, which now includes padding on the x axis. The only way to remove it is to modify the token value, which also can't be done through inline styles, because ExpandableRowContent props only accept children. We might want to extend the props on the React side, or even provide a direct prop to remove the x-axis padding?

V6 example:
Image

@adamviktora
Copy link
Collaborator

Opened a design issue for the above ^

Won't migrate this component, closing.

@github-project-automation github-project-automation bot moved this from In Progress to Done in PatternFly Issues Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants