-
Notifications
You must be signed in to change notification settings - Fork 503
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
fix(tables): wrap in table-container-inner #9974
base: main
Are you sure you want to change the base?
Conversation
build/wrap-tables.ts
Outdated
const figure = $( | ||
'<figure class="table-container"><figure class="table-container-inner"></figure>' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the solution might actually be to add a CSS class for these tables that DON'T have the nested table-container-inner
:
const figure = $( | |
'<figure class="table-container"><figure class="table-container-inner"></figure>' | |
); | |
const figure = $( | |
'<figure class="table-container"><figure class="table-container-inner"></figure>' | |
); |
const figure = $( | |
'<figure class="table-container"><figure class="table-container-inner"></figure>' | |
); | |
const figure = $('<figure class="table-container standalone"></figure>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost ready
👍 BCD table got its own scroll, all tables OK.
tested page :- http://localhost:3000/en-US/docs/Web/HTML/Element/input
margin: 0 -3rem; | ||
overflow: auto; | ||
width: 100vw; | ||
width: 100vw !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My personal opinion, don't need to overwrite width because .table-container
width is 100% applying to all tables. And we don't need view port width. Remove it.
@@ -25,7 +25,7 @@ | |||
} | |||
} | |||
|
|||
.table-container { | |||
.bc-table-outer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mobile view needs following changes
.bc-table-outer { | |
.table-container{ | |
overflow-x: auto; | |
} |
mobile_issue.mp4
This pull request has merge conflicts that must be resolved before it can be merged. |
Summary
Fixes #9962.
Problem
Tables in blog articles were overflowing on sm/md/lg screens, because they were only wrapped in
figure.table-container
, but not infigure.table-container-inner
(like the BCD table).Solution
Wrap tables in
figure.table-container > figure.table-container.inner
.Screenshots
How did you test this change?
Ran
yarn && yarn dev
and opened http://localhost:3000/en-US/blog/vs-code-tips-tricks/#shortcuts_for_navigation locally (requires mdn/mdn-studio).