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-responsive regression #24256

Closed
alecpl opened this issue Oct 5, 2017 · 5 comments
Closed

.table-responsive regression #24256

alecpl opened this issue Oct 5, 2017 · 5 comments
Assignees
Labels

Comments

@alecpl
Copy link
Contributor

alecpl commented Oct 5, 2017

After #23665 I see one problem. A table with table table-responsive class does not fit the screen width on a big screen. Previously the display:block was added only on small widths, now it applies always.

For your consideration if it's worth to change or it's as expected, but note it's a regression.
I'll be fine with any decision.

@pat270
Copy link
Contributor

pat270 commented Oct 6, 2017

I think this bug appeared when .table-responsive was moved to the table element. The quickest fix is to go back to the way Bootstrap 3 did it and wrap the table in <div class="table-responsive"></div>.

@andresgalante
Copy link
Collaborator

andresgalante commented Oct 7, 2017

Hey @alecpl Thanks a lot for reporting this.

Responsive tables are extremely hard to solve on a component level since there is no universal solution for them. It all depends on the data the user has to display. I like to call them the "holy grail" of components 😄. Having said that I don't see any usecase that can benefit from the docs introduced on #23665.

To me it generates unclear documentation and comments. Duplicates width: 100% that's already defined on.table and uses max-width media queries which aren't ideal on a mobile first framework.

While reviewing this issue I also noticed 2 things: Our docs have a warning about overflow-y for .table-responsive that doesn't exist on the code. And we are setting width: 100%; max-width: 100%; on .table which can probably be just min-width: 100%.

@mdo @XhmikosR What do you guys think? If there is a really good reason to keep the table-reponsive-* classes I am more than happy to clean it up and write better docs. Otherwise I'd just remove them and review tables docs and scss.

@XhmikosR
Copy link
Member

@andresgalante: I'd say this is purely @mdo's choice. If it were up to me I'd keep .table-responsive and just fix the docs.

@mdo
Copy link
Member

mdo commented Oct 18, 2017

Yup, let's any docs inaccuracies or shortcomings. We can also call out max-width vs min-width nearly everywhere else.

@mdo
Copy link
Member

mdo commented Oct 19, 2017

PR at #24438 improves docs for this by including demos and better explaining how they work.

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

No branches or pull requests

6 participants