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.

Sign up for GitHub

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

Add top margin to nested module prefix in sidebar #1929

Merged
merged 1 commit into from
Jul 2, 2024
Merged

Add top margin to nested module prefix in sidebar #1929

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions assets/css/sidebar.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@
font-size: 0.9em;
line-height: 1.8em;
color: var(--sidebarSubheadings);
margin-top: 10px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The grouping functionality uses 1.5em. Should we use the same?

Suggested change
margin-top: 10px;
margin-top: 1.5em;

Copy link
Contributor Author

@zachallaun zachallaun Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with that if you'd prefer. I personally think it errs on the side of too much margin, but I don't feel strongly. Here's what that looks like:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the grouping functionality: that font size is .8em whereas the module nesting uses .9em, so 1.5em margin ends up being a bit larger for module nesting. To make them consistent, we could update module nesting to use .8em and 1.5em margin.

Copy link
Contributor Author

@zachallaun zachallaun Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Sorry for the repeated messages.) Looking at it now, bumping the nested modules down to .8em seems a bit too small. The other grouping uses ALL CAPS, so it's more readable at that smaller size.

I'll ultimately defer to you.

image

padding-left: 15px;
}

Expand Down
Loading