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.

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

Version selector #500

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Version selector #500

wants to merge 8 commits into from

Conversation

waynew
Copy link

@waynew waynew commented Aug 5, 2022

I definitely plan to do a rebase to clean up this commit history (probably end out just squashing to one commit) before I'm done with this as a draft status.

But this is my PoC for #372, and the instructions I dropped in there should work here as well.

The things that I'm pretty sure I should do differently are around where to put my styles, as well as perhaps some of the naming(s) that I used. I can possibly do some cleanup with the code/jinja in variant-selector (obviously there's still some debugging code in there that'll need to be removed).

Here's what it currently looks like with the latest version selected:

image

And with a different version selected:

image

One thought I had was to change up the behavior a bit to make it match with the drop-down styling found in the top bar. I also just now thought it looked a bit busy with the list, and I kind of like it using a list-style: none:

image

Any thoughts?

waynew added 6 commits July 22, 2022 16:14
This "works" with sphinx-multiversion.

It's very ugly, but the versions *will* show up in the sidebar if we're
not being built on RTD. From Salt's perspective we probably want to just
keep with the recent approach of master/main/latest, and then adding the
previous two versions. Or maybe even just limiting build to the
supported versions?
Tried to match the styles in the main navbar, and just bolding+list
style changing the selected version. It seems to be pretty decent?
@eviau-sat
Copy link
Contributor

eviau-sat commented Sep 3, 2022

Just tested this version selector - it seems to be working fine ?

I am on Firefox and Ubuntu, and I have a similar design as the one you are showing. Personally I prefer the :list-style: none version, and I do not have good arguments to say so - it just feels less busy ?

I was curious to know if you have a preference for list or dropdown menu - I see how there could be many more versions to be added in the future, I am not sure what's a good way to manage this yet.

If you have other things you wanted thoughts/comments/tests on, let me know :)

PS: maybe it would be wiser to post comments on the original issue... ?

@pradyunsg
Copy link
Owner

pradyunsg commented Sep 9, 2022

Any thoughts?

None other than that I'm happy to see this PR, and there's a bunch of styling improvements needed to make this mirror the existing sidebar contents.

I think I'd prefer to mirror the way the RTD version selector shows up in the sidebar; collapsed with only the current version visible in the sidebar until you hover or tap on it; which expands it and shows you the entire listing.

I'm not a fan of the arrow for pointing out the current version, nor the lack of spacing between the various bullet points -- I'd prefer if we change those to look like the toctree or like the RTD version listing.

@waynew waynew marked this pull request as ready for review October 31, 2022 16:36
@waynew waynew changed the title [WIP] Version selector Version selector Oct 31, 2022
@waynew
Copy link
Author

waynew commented Oct 31, 2022

Hey @pradyunsg we finally got all the stars aligned and I was able to get this updated. Here's what the new changes look like:

Collapsed
image

Expanded
image

Let me know if this is 🚀 or if there's anything that we should change!

@waynew
Copy link
Author

waynew commented Nov 8, 2022

@pradyunsg no huge rush, but do you have any idea what your timeline is for reviewing this? Thanks!

@pradyunsg
Copy link
Owner

I'll try and take a look this weekend. :)

@pradyunsg
Copy link
Owner

pradyunsg commented Nov 16, 2022

Actually, just looking at the screenshots: I think it'd be cleaner/better to mirror the style + behaviour that we have for version/build information on RTD today (see https://furo--500.org.readthedocs.build/ for an example). I'll try and poke around with this PR over the weekend tho.

Let me know if you reckon that might be a good thing to do, or have an opinion that it's not cleaner/better. :)

@mscheltienne
Copy link
Contributor

Hello all! First of all, thank you for adding this feature! I've been following along for a couple of months now. waiting eagerly for its release. I'd like to chip in and offer my opinion, as a user of Furo and pydata-sphinx-theme.


There are 2 aspects I dislike about the version selector from readthedocs:

  • it's too discrete (too small, too low on the page as its pushed to the very bottom left corner),
  • it's non-persistent and disappears as soon as you drag the mouse outside its area of effect.

It's also why I like the version selector from pydata-sphinx-theme, e.g. on scipy: it's visible and persistent once you unfold it (until you fold it again or click outside of its area of effect).


For the proposal by @waynew, from the screenshots, it looks persistent and more visible than its readthedocs counterpart (it's positioned as a menu, higher up in the left column). The one amelioration I would add is an indicator of the currently selected version when the menu is folded.

Here are my 2c, from a user who likes the style from Furo 😄

@waynew
Copy link
Author

waynew commented Dec 14, 2022

Are there any concrete changes that we need to make on this end? Or @pradyunsg did you still want to take a closer look?

They were there for testing, and we missed removing them. They're gone
now.
@barbaricyawps
Copy link

@pradyunsg , is there any way we can get this merged in?

We are actually already using the version selector developed by @waynew at the Salt Project. You can see it in action because we're currently building our docs from a fork of Wayne's merge.

Check it out in action by going first to the latest version of the Salt Install Guide, then click 3004 from the version selector in the lower left menu to go to the 3004 version of the docs. If it's working correctly (and it should be), the 3004 version will show a warning message at the top indicating it's an older version of the docs.

If this could get merged in, that would make it so that we could just update our Furo version rather than building the theme from a fork. That would be nice, if possible, but no rush. 😄

@mscheltienne
Copy link
Contributor

+1 for this, looks super nice on your doc and looking forward to integrating it in my projects!

@barbaricyawps
Copy link

Oh, and if you need a docs contribution to explain how to make it work, I can volunteer to do that!

@pradyunsg
Copy link
Owner

This is quite high up on my "things to do in open source" list. It's also got a few higher priority items like engaging in the packaging strategy discussions and cutting pip releases that have meant that I've not spent much time toward this. 😅

@barbaricyawps
Copy link

That sounds just fine to me! Thanks so much for your help, for all you do in open source generally, and for being so willing to accept this contribution!

@eviau-sat
Copy link
Contributor

Just wanted to up this PR for now - we could use it for media art docs - e.g. antimodular/Best-practices-for-conservation-of-media-art#7 and antimodular/Best-practices-for-conservation-of-media-art#4

Still happy to help test this out !

@barbaricyawps
Copy link

Hi, @pradyunsg , I was just curious if we could get a status update on this PR and whether it's in a merge-ready state or not? Thanks in advance!

@waynew
Copy link
Author

waynew commented Oct 7, 2023

@pradyunsg just another ping here 😇

@pradyunsg pradyunsg added the configuration Affects one or more configurable behaviours label Apr 27, 2024
@AVHopp
Copy link

AVHopp commented Apr 29, 2024

Hi @pradyunsg also just wanted to ask if there is a chance of this being merged soon 😇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Affects one or more configurable behaviours
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants