-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Scrollbar disappears when sidebar is short #90943
Labels
A-rustdoc-ui
Area: Rustdoc UI (generated HTML)
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
Comments
jsha
added
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
A-rustdoc-ui
Area: Rustdoc UI (generated HTML)
labels
Nov 16, 2021
I think this is a good idea. I'll send a PR for it. |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Nov 20, 2021
…=jsha Make scrollbar in the sidebar always visible for visual consistency Fixes rust-lang#90943. I had to add a background in `dark` and `ayu` themes, otherwise it was looking strange (like an invisible margin). So it looks like this: ![Screenshot from 2021-11-17 14-45-49](https://user-images.githubusercontent.com/3050060/142212476-18892ae0-ba4b-48e3-8c0f-4ca1dd2f851d.png) ![Screenshot from 2021-11-17 14-45-53](https://user-images.githubusercontent.com/3050060/142212482-e97b2fad-68d2-439a-b62e-b56e6ded5345.png) Sadly, I wasn't able to add a GUI test to ensure that the scrollbar was always displayed because it seems not possible in puppeteer for whatever reason... I used this method: on small pages (like `lib2/sub_mod/index.html`), comparing `.navbar`'s `clientWidth` with `offsetWidth` (the first doesn't include the sidebar in the computed amount). When checking in the browser, it works fine but in puppeteer it almost never works... In case anyone want to try to solve the bug, here is the puppeteer code: <details> More information about this: I tried another approach which was to get the element in `evaluate` directly (by calling it from `page.evaluate(() => { .. });` directly instead of `parseAssertElemProp.evaluate(e => {...});`. ```js const puppeteer = require('puppeteer'); (async () => { const browser = await puppeteer.launch(); const page = await browser.newPage(); await page.goto("file:///path/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/doc/lib2/sub_mod/index.html"); await page.waitFor(".sidebar"); let parseAssertElemProp = await page.$(".sidebar"); if (parseAssertElemProp === null) { throw '".sidebar" not found'; } await parseAssertElemProp.evaluate(e => { const parseAssertElemPropDict = {"clientWidth": "192", "offsetWidth":"200"}; for (const [parseAssertElemPropKey, parseAssertElemPropValue] of Object.entries(parseAssertElemPropDict)) { if (e[parseAssertElemPropKey] === undefined || String(e[parseAssertElemPropKey]) != parseAssertElemPropValue) { throw 'expected `' + parseAssertElemPropValue + '` for property `' + parseAssertElemPropKey + '` for selector `.sidebar`, found `' + e[parseAssertElemPropKey] + '`'; } } }).catch(e => console.error(e)); await browser.close(); })(); ``` </details> r? `@jsha`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-rustdoc-ui
Area: Rustdoc UI (generated HTML)
T-rustdoc
Relevant to the rustdoc team, which will review and decide on the PR/issue.
In this example, for ToString, the sidebar is shorter than the page, so there is no scrollbar:
In this example, for String, the sidebar is longer so there is a scrollbar:
The problem here is that the scrollbar forms part of the visual layout of the page, separating the sidebar from the main content. I think for the sake of consistency we should always show the sidebar.
The text was updated successfully, but these errors were encountered: