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

Jump to bottom

rustdoc ui: adjust tooltip z-index to be above sidebar #119477

Merged
merged 2 commits into from
Jan 3, 2024
Merged

rustdoc ui: adjust tooltip z-index to be above sidebar #119477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d796ad4
rustdoc ui: adjust tooltip z-index to be above sidebar
Dec 31, 2023
b1853eb
use css variable for z-index of the sidebar
Dec 31, 2023
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
10 changes: 5 additions & 5 deletions src/librustdoc/html/static/css/rustdoc.css
View file
Open in desktop
Copy link
Member

@fmease fmease Jan 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Btw, have you verified that flattening the z-index stack from (0, 2, 100, 101, 200) to just (0, 100, 101) doesn't incur further overlap regressions? @lukas-code

Copy link
Member Author

Choose a reason for hiding this comment

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

For the .sidebar-resizer i checked that it still looks correct for changing 200 to 101. The .popover going from 2 to 101 is exactly the bug this PR is trying to fix and I can't think of anything that's supposed to go over the popover at the moment.

Additionally, changing the resizer to 101 makes it appear below the popover, which seems more correct to me:

image

Alternatively it would look like this if we leave the resizer at 200:

image

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for double-checking, your version does indeed look better.

Copy link
Member Author

@lukas-code lukas-code Jan 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Apparently this also affects the order of the settings/help popups and the sidebar while resizing the sidebar in a really narrow window. I don't know if this is relevant enough to add a special case.

before:

2024-01-01_16-40-09.mp4

after:

2024-01-01_16-40-24.mp4

Copy link
Contributor

@notriddle notriddle Jan 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Clicking the sidebar to resize it should just close the Settings popover. Make this whole discussion a moot point.

That's not important right now, though. I'll make a separate PR to fix this after it merges.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
and on the RUSTDOC_MOBILE_BREAKPOINT */
--desktop-sidebar-width: 200px;
--src-sidebar-width: 300px;
--desktop-sidebar-z-index: 100;
}

/* See FiraSans-LICENSE.txt for the Fira Sans license. */
Expand Down Expand Up @@ -386,7 +387,7 @@ img {
height: 100vh;
top: 0;
left: 0;
z-index: 100;
z-index: var(--desktop-sidebar-z-index);
}

.rustdoc.src .sidebar {
Expand All @@ -407,7 +408,7 @@ img {
touch-action: none;
width: 9px;
cursor: col-resize;
z-index: 200;
z-index: calc(var(--desktop-sidebar-z-index) + 1);
position: fixed;
height: 100%;
/* make sure there's a 1px gap between the scrollbar and resize handle */
Expand Down Expand Up @@ -439,7 +440,6 @@ img {

.sidebar-resizing .sidebar {
position: fixed;
z-index: 100;
}
.sidebar-resizing > body {
padding-left: var(--resizing-sidebar-width);
Expand Down Expand Up @@ -1046,7 +1046,7 @@ so that we can apply CSS-filters to change the arrow color in themes */
position: absolute;
top: 100%;
right: 0;
z-index: 2;
z-index: calc(var(--desktop-sidebar-z-index) + 1);
margin-top: 7px;
border-radius: 3px;
border: 1px solid var(--border-color);
Expand Down Expand Up @@ -1561,7 +1561,7 @@ a.tooltip:hover::after {
}
.src #sidebar-button {
left: 8px;
z-index: 101;
z-index: calc(var(--desktop-sidebar-z-index) + 1);
}
.hide-sidebar .src #sidebar-button {
position: static;
Expand Down
12 changes: 12 additions & 0 deletions tests/rustdoc-gui/tooltip-over-sidebar.goml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Check that the doctest info tooltips are above the sidebar.
go-to: "file://" + |DOC_PATH| + "/test_docs/fn.foo.html"
move-cursor-to: ".example-wrap.ignore .tooltip"
wait-for: ".tooltip.popover"

// Move cursor to top left corner of the tooltip and check that it doesn't fade.
move-cursor-to: ".tooltip.popover"
wait-for: 100
assert: ".tooltip.popover:not(.fade-out)"

move-cursor-to: (0, 0)
wait-for: ".tooltip.popover.fade-out"
Loading