Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Make the sidebar resizable#214

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
tomasr8 wants to merge4 commits intopython:mainfromtomasr8:resizable-sidebar

Conversation

tomasr8
Copy link
Member

@tomasr8tomasr8 commentedJan 25, 2025
edited by hugovk
Loading

Addresses#209

  • Sidebar is no longer collapsible but is instead resizable with a slider
  • Bumps the width to250px (happy to bikeshed the width)
  • Uses flexbox for the sidebar content and sidebar button instead of floats
  • Adds some ARIA-specific attributes to the slider

Feedback welcome!

Preview:https://python-docs-theme-previews--214.org.readthedocs.build/en/214/

Copy link
Member

@hugovkhugovk left a comment

Choose a reason for hiding this comment

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

I wonder if we should have a min and/or max width, or let the user have a tiny or massive sidebar if they really want?

I will note that PR this does make it impossible to completely collapse the sidebar, which is possible by clicking the divider athttps://docs.python.org/3/. Is that something we want to keep?

@tomasr8
Copy link
MemberAuthor

tomasr8 commentedJan 26, 2025
edited
Loading

I wonder if we should have a min and/or max width

I agree, it might be nicer to have some limits. I'll add some so we can try it out

I will note that PR this does make it impossible to completely collapse the sidebar, which is possible by clicking the divider athttps://docs.python.org/3/. Is that something we want to keep?

I removed it because I felt it wouldn't really have much of a purpose anymore, though if people prefer both we could make it work :)

@tomasr8
Copy link
MemberAuthor

I added a min/max width for the sidebar (200px minimum and a half ofwindow.innerWidth maximum but we can refine the values)

@tomasr8tomasr8 requested a review fromhugovkJanuary 26, 2025 22:25
@tomasr8tomasr8 requested a review fromhugovkJanuary 27, 2025 20:53
@hugovk
Copy link
Member

Left: with JS
Right: without JS, still showing the "«" symbol, making it look collapsible, but it's not. Can we remove it?

image

@tomasr8
Copy link
MemberAuthor

Right: without JS, still showing the "«" symbol, making it look collapsible, but it's not. Can we remove it?

Definitely, I need to get into the mindset that JS might not be available 😅 Currently I'm removing it through JS, but a CSS rule should work even better. I'll fix it later today

hugovk reacted with thumbs up emoji

@ezio-melotti
Copy link
Member

ezio-melotti commentedJan 28, 2025
edited
Loading

I'm not sure making the sidebar resizable is the right solution to the problem described in#209.

The problem I was solving when I made the sidebar collapsible many years ago was to improve the experience on small screen (mostly phones), where the sidebar was taking up over half of the screen. Making it collapsible is also useful on regular screens when the user doesn't need it and wants more space for the actual documentation.

This PR takes away the collapsibility and replaces it with resizeability which is IMO much less useful.

Going back to the original problems listed in the issue:

  1. Content doesn't always fit
    • I think a better solution for this would be to use CSS ellipsis possibly combined with atitle="" or something similar to show the full text on hover (even though last time I checked there wasn't an easy way to achieve this with CSS only). I'd also argue that this is functionally a non-issue and at best an aesthetic one, since the visible part is most often enough to recognize what you are looking for (assuming you are not already usingctrl+f). Changingoverflow and/orword-wrap rules, increasing sidebarwidth, decreasingtext-indent, reducingfont-size are also viable solutions.
  2. Sidebar is too narrow on wide screens
    • This could be solved either with a%-based width, or with media queries

I'd would be ok with a resizable sidebar if it didn't affect collapsibility, but offering both features might just end up complicating the UI for little gain IMO. Also note that dragging is more difficult and error-prone than just tapping to (un)collapse, especially on touchscreens.

@hugovk
Copy link
Member

The problem I was solving when I made the sidebar collapsible many years ago was to improve the experience on small screen (mostly phones), where the sidebar was taking up over half of the screen.

At least since the theme was made responsive (in#46) the sidebar is hidden on screens narrower than 1024 pixels, so this part shouldn't be a problem any more.

Going back to the original problems listed in the issue:

I'm more favourable to making the sidebar wider than trying to display more stuff in a small space. Screens are getting wider.

Truncating text has usability/accessibility issues (can't ctrl+f), as does showing text on hover (can't hover on touch devices, there's moreaccessibility concerns).

ezio-melotti and tomasr8 reacted with thumbs up emoji

@ezio-melotti
Copy link
Member

Truncating text has usability/accessibility issues (can't ctrl+f), as does showing text on hover (can't hover on touch devices, there's moreaccessibility concerns).

Note however that if you arectrl+fing something:

  1. even if it doesn't find it in the sidebar, it will find it directly in the page (which is even better).
  2. if the name is so long that it doesn't fit in the sidebar, chances are that you aren't going to type it all before getting a match (or failing to get one in the sidebar because you typed the part that doesn't fit).

I also checked theexample linked in the original issue and:

  • the sidebar already seems to implementoverflow-wrap: break-word, solving the truncation problem;
  • the depth of the sidebar seems to have been reduced, so that -- at least in that page -- there are no overflowing words anymore.

sidebarbutton.role = "slider"
sidebarbutton.title = _("Resize sidebar")
sidebarbutton.style.cursor = "col-resize" // Set the cursor only if JS is enabled
sidebarbutton.setAttribute("aria-label", _("Resize sidebar by dragging"))

Choose a reason for hiding this comment

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

Noting thataria-label should only be used like so to provide additional usage context to assistive technology users.
After testing this it seems the resizing function is only operable via dragging (mouse or similar) so it seems thisaria-label is used as a "catch all" to provide usage instructions, thus defeating the purpose of ARIA, it might be best removing it altogether, and instead add a comment.

window.localStorage.setItem("sidebar", "collapsed")
}
sidebarbutton.innerHTML = ""
sidebarbutton.tabindex = "0" // make it focusable

Choose a reason for hiding this comment

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

Noting that I could not focus this while navigating with a keyboard nor through screen reader so there might be something tripping this.

@trallard
Copy link

I agree with@ezio-melotti here, I am not convinced this approach addresses the issues from the original issue. I think a much simpler alternative would be to make the sidebar wider (with a minimum width or similar) while keeping the collapsing function as this is a feature folks used when multitasking or in smaller screens.

The slider/splitter added in this PR adds a lot more complexity, and is only usable with a mouse so adds a new layer of accessibility concerns. There would be significant changes needed to make it keyboard and assistive tech operable and we might need keeping the collapsing feature anyway leading to a complex UI which might prove tricky to get right.

(Note that the collapse button as is needs some rework to make it operable with multiple inputs, but we can work on that separately but would take a lot less work than making an accessible slider + accessible collapse button).

hugovk, tomasr8, and ezio-melotti reacted with thumbs up emoji

@tomasr8
Copy link
MemberAuthor

Noting that I could not focus this while navigating with a keyboard nor through screen reader so there might be something tripping this.

Strange, it's working fine for me locally. Maybe some issue with the preview?

Anyway, thanks for your input! I see your points and I agree that adding a slider might be too disruptive to some workflows. Personally, I'd also be happy with just increasing the (max-)width of the sidebar so that it's a bit more flexible.

[...] this is a feature folks used when multitasking or in smaller screens.

Out of curiosity, what kind of set up are we talking about? The sidebar is hidden for widths <1024 pixels and for screens larger than that there's enough space for the text even with the sidebar.

In any case, if nobody disagrees, I can close this PR and open one that simply makes the sidebar wider :)

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@trallardtrallardtrallard left review comments

@hugovkhugovkAwaiting requested review from hugovk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@tomasr8@hugovk@ezio-melotti@trallard

[8]ページ先頭

©2009-2025 Movatter.jp