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

Table/Empty state: the Empty State table from the react demos is breaking on mobile #5379

Closed
KKoukiou opened this issue Jan 27, 2021 · 6 comments · Fixed by #5886
Closed
Assignees
Milestone

Comments

@KKoukiou
Copy link
Collaborator

KKoukiou commented Jan 27, 2021

The examples from the docs page looks like this in mobile:

Screen Shot 2021-01-27 at 13 42 54

@KKoukiou KKoukiou changed the title Table/Empty state: the Empty State table from the react demos is breaking on mobile screen sizes Table/Empty state: the Empty State table from the react demos is breaking on mobile Jan 27, 2021
KKoukiou added a commit to KKoukiou/cockpit that referenced this issue Jan 27, 2021
* storage: remove table-hover class from empty tables
* machines: there needs to be at least one column defined for the table empty state to show up
* lib: add css workaround for PF4 broken empty states in tables
    See patternfly/patternfly-react#5379

Fixes cockpit-project#15213
martinpitt pushed a commit to cockpit-project/cockpit that referenced this issue Jan 28, 2021
* storage: remove table-hover class from empty tables
* machines: there needs to be at least one column defined for the table empty state to show up
* lib: add css workaround for PF4 broken empty states in tables
    See patternfly/patternfly-react#5379

Fixes #15213
@tlabaj tlabaj added this to the Prioritized Backlog milestone Jan 29, 2021
thomasvandenbosch13 pushed a commit to thomasvandenbosch13/cockpit that referenced this issue Apr 12, 2021
* storage: remove table-hover class from empty tables
* machines: there needs to be at least one column defined for the table empty state to show up
* lib: add css workaround for PF4 broken empty states in tables
    See patternfly/patternfly-react#5379

Fixes cockpit-project#15213
@kmcfaul
Copy link
Contributor

kmcfaul commented May 10, 2021

What view would you expect for the empty state demo on mobile @KKoukiou @mcarrano? This demo is inconsistent in that other table demos on a mobile size list all columns on the left per row. But I can also see a case for not displaying the columns at all and having the empty state take up 100% of the space for mobile.

Table doesn't have a notion of 'empty state' at the moment. It only sees the columns and rows that are passed in, and the empty state is passed as a single item in a row. It only looks like the empty state takes up the whole table space due to styling, and this causes the side-effect currently seen on mobile (because the empty state is only associated with a single column).

If columns should be hidden on mobile sizes, the solution would be to conditionally pass in an array containing an empty string (['']), or possibly to add a flag to hide the columns.

@nicolethoen Am I thinking through this correctly?

@mcarrano
Copy link
Member

I have to admit that this is a bit odd. In the Empty state page we have this example that represents better what I would expect: https://www.patternfly.org/v4/components/empty-state#no-match-found So in thins case there are no table headers at all and the table is just replaced with the empty state component.

@kmcfaul
Copy link
Contributor

kmcfaul commented May 10, 2021

Our HTML demo for Table does keep the columns with the empty state, so I'm slightly confused. Do you mean the Table should only show the EmptyState without columns when the screen is small or at all times?

https://www.patternfly.org/v4/components/table/html-demos/empty-state/

@mcoker
Copy link
Contributor

mcoker commented May 11, 2021

We just need to be able to remove the data-label="Servers" attribute from the <td> that holds the empty state component.

@mcarrano
Copy link
Member

Thanks for the tip @mcoker . I've added this to our roadmap to fix.

@kmcfaul
Copy link
Contributor

kmcfaul commented May 12, 2021

I think we'll need a flag to indicate that a cell is an empty state. That way the table will know to not associate it with a column (the data-label).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants