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

.markdown-body table styling #1127

Closed
pbb72 opened this issue Jul 22, 2020 · 5 comments · Fixed by #1128
Closed

.markdown-body table styling #1127

pbb72 opened this issue Jul 22, 2020 · 5 comments · Fixed by #1128
Labels
Stale Automatically marked as stale.

Comments

@pbb72
Copy link

pbb72 commented Jul 22, 2020

Describe the bug
The styles that are being applied to tables within .markdown-body are not doing what they seem to be meant to do, and are hindering layout with table borders.

Tables are given both a display: block and a width: 100%:

.markdown-body {
// Tables
table {
display: block;
width: 100%;

I am not sure if these rules were supposed to fix anything, or if they are just the result of bad testing. The width rule would have made sense if the table was still a table, but it does not here as block defaults to full width already. Forcing the table to display as block breaks the table layout, as the containing elements (namely tbody) still render as table elements.

To Reproduce
Steps to reproduce the behavior:

  1. On GitHub, create a document in a markup language that allows adding table borders, for example .wiki

  2. Create a table with a border:

    {| border="2"
    |-
    | A1 || A2
    |-
    | B1 || B2
    |}
    
  3. See error, the border spans the whole document width, but the table contents sits to the left.

Expected behavior
The border should wrap nicely around the table

Screenshots
Actual result:
image

Expected result: either
image
or
image

Suggested fix:
At the very least, remove the display: block rule, and if possible also the width rule.

@vdepizzol
Copy link
Contributor

@pbb72 Hello Peter. It looks like the display: block, width: 100% and overflow: auto properties are used to guarantee the table will scroll horizontally when there's not enough space (for mobile scenarios, or for large tables). It seems that we overlooked how Wiki could support custom borders.

@simurai I wonder if changing from width to max-width could solve this problem while keeping the scrolling mechanism?

@simurai
Copy link
Contributor

simurai commented Jul 23, 2020

are used to guarantee the table will scroll horizontally

👍 Yeah, right. We tried to remove display: block to make tables more accessible, see #924, but couldn't find a way to make them horizontally scrollable for very long strings like in the table below:

Before After
asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf asdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdfasdf

I wonder if changing from width to max-width could solve this problem while keeping the scrolling mechanism?

Yes, it seems that a combo of width: max-content; max-width: 100%; would work. 👌

table

@pbb72
Copy link
Author

pbb72 commented Jul 23, 2020

That looks like a good solution.

Curiously, just having the following (no width-related rules at all) also seems to do the trick:

.markdown-body table {
  display: inline-grid;
}
.markdown-body tbody {
  overflow: auto;
}

But I am not skilled enough in grids yet to see if that could cause other problems...

@simurai
Copy link
Contributor

simurai commented Jul 24, 2020

Not sure either. 🤔 Using grid should have enough browser support by now, but maybe still nice to keep the width: 100% as fallback for the few users that use an older browser. 🤷

@github-actions
Copy link
Contributor

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

@github-actions github-actions bot added the Stale Automatically marked as stale. label May 26, 2021
@github-actions github-actions bot closed this as completed Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale Automatically marked as stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants