-
Notifications
You must be signed in to change notification settings - Fork 21
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
Remove unnecessary scroll bars from specification tables #424
Conversation
e9ab17e
to
b388ba8
Compare
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.
done self-reviewing
@@ -47,7 +47,7 @@ export const RequestedPackageList = ({ | |||
</StyledAccordionSummary> | |||
<StyledAccordionDetails sx={{ padding: 0 }}> | |||
<TableContainer> | |||
<Table sx={{ width: "420px", tableLayout: "fixed" }}> | |||
<Table sx={{ width: "100%", tableLayout: "fixed" }}> |
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 I previously thought that with table-layout: fixed
, I needed to set a concrete width in pixels, but the MDN article on table-layout suggests that a percentage value for the width is fine. The key point is two-fold:
- Must set width to something (cannot leave unspecified)
- Except must not set width to
auto
In order for the browser to respect table-layout: fixed
and not use table-layout: auto
instead.
@@ -15,8 +15,8 @@ export const StyledAccordionDetails = styled(AccordionDetails, { | |||
? `1px solid ${palette.secondary.light}` | |||
: "1px solid #BCBFC4", | |||
borderTop: "none", | |||
borderRadius: styleType === "grayscale" ? "0px 0px 5px 5px" : "Opx", | |||
overflowY: "scroll", | |||
borderRadius: styleType === "grayscale" ? "0px 0px 5px 5px" : "0px", |
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.
The character on the left is not actually the zero numeral
borderRadius: styleType === "grayscale" ? "0px 0px 5px 5px" : "Opx", | ||
overflowY: "scroll", | ||
borderRadius: styleType === "grayscale" ? "0px 0px 5px 5px" : "0px", | ||
overflowY: "auto", |
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.
Don't show the vertical scroll bar unless there is actually overflow
b388ba8
to
ea41633
Compare
Before
After
No more unnecessary scroll bars.