-
Notifications
You must be signed in to change notification settings - Fork 484
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
Sidebar updates #409
Sidebar updates #409
Conversation
Looks pretty good—works well on macOS/Safari. I might prefer it if the dropdown menu and default text in the search box (and the logo) was all centered... On iPad (and probably iPhone too but...) the navigation bar isn't very usable, though —touch event handling or something. Perhaps that could be added as a separate issue... |
The selector doesn't display the dropdown arrow as far as see. Could we add that back since it's a useful indication of what it is.
|
Yup, I agree that it would be nice to have. However, I removed it intentionally since it can't be styled and it is very inconsistent across platforms/browsers and doesn't really match with the rest of the style (especially in Firefox). But I should play around with it a bit more -- I think the standard workaround is to use background images on the
It would be nice if it wouldn't depend on javascript though and the current version would be displayed statically. But this sounds like a reasonable solution for the first iteration. |
Everything except the text in the search box should be centered. Could you give me a screenshot of what you're seeing?
Mobile experience from both the layout and touch experience point of view has not received any attention so far really. Would definitely be something that could be improved. |
Looks alright for me on Firefox, I think it depends on what dm/wm and theme is being used.
There's already lots that depends on js in the docs, so for a first pass it's fine to require it for this.
According to the site analytics mobile views account for less than 5% of all traffic to Documenter's docs, so probably not a critical requirement just yet. |
For me on Firefox, when I re-enable the arrow, it looks like this: However, I agree that there should be something indicating that you can actually select a version there, so I'll look into that. Also, agreed, let's go with a As for centering -- the The search box I would keep left-aligned since it's quite wide and then it matches the menu below. Just FYI: I'll be travelling in the coming days though, so this PR will likely be on hold until the latter half of next week. |
Looks like a 0.4-specific failure. You're welcome to drop 0.4 support if you'd like to — we've been keeping it going for long enough I think. |
It's a small type thing with |
Updated on my part, latest build should be here (built and deployed on Travis). Changes:
I do feel that this needs more work in the long run. Generating |
Cleaned up and rebased, latest build updated. I would say that it's ready to merge. Only thing unaddressed is the Safari style, which I can't test myself. @cormullion ? |
The text in the selector should be centered (and is on other platforms). What did you add before to make it centered in Safari, @cormullion? |
My artist's impression above, you mean? That was image editing... I moved the stuff around manually. That was after a quick and unsuccessful look at the Safari Inspector thing, which lets you fiddle with the HTML/CSS. (I think Safari on iPad and Mac will be the same. I'll have a look after breakfast...☕️🥐) |
Ah, I thought you did it in the inspector. I added a note about this for future reference as it looks OK even if it is not centered. |
Increase the contrast of the text in the sidebar by: - making the text near-black, same as the main body - make the :hover text on dark background light (same as nav.toc background)
Add a version argument to makedocs that will allow specifying the version shown in the selector by default. If it is not specified then the version selector will be empty and have visibility: hidden so that an empty selector wouldn't show up. Using visibility: instead of display: so that the sidebar wouldn't jump when the selector gets enabled. Separate the version selector javascript into a separate requirejs block. Amend the version selector javascript so that existing selector options get checked and updated, if necessary. A siteinfo.js file will now also be generated in the root of the build when being deployed which can be used to determine the current version. Move version selector out of search <form> tag, making it easier to style and since it is not conceptually related to the search. Update the style of the selector to be uniform across browsers, more in line with the style and more connected to the package name rather than the search box. The SVG arrow is necessary to make the dropdown indicator uniform.
Style and behaviour updates for the sidebar. Site build available here.
The WIP bit is to make
makedocs
automatically determine the current version name (e.g.latest
, tag name) which is currently only done indeploydocs
(so, right now it would just show the commit hash for all builds).I am also not necessarily entirely happy with the version selector's style, so feedback & ideas would be greatly appreciated (cc @cormullion).
Ideally fixes #406.