Skip to content
This repository has been archived by the owner on Dec 5, 2017. It is now read-only.

Change the width of the sidebar. #13

Merged
merged 2 commits into from
Jul 15, 2014
Merged

Conversation

bpindelski
Copy link

This should help avoid situations where the sidebar overflows above the main text:

screen shot 2014-07-11 at 14 04 08

@bpindelski bpindelski changed the title Change the width of the sidebar to "auto". Change the width of the sidebar. Jul 11, 2014
@bpindelski
Copy link
Author

/cc @mtbc, @pwalczysko, @hflynn, @rleigh-dundee, @gusferguson

This change will need a consensus between all involved and with an opinion.
It's a fact that we're using the sphinxdoc theme that doesn't have support for arbitrary positioning of the sidebar (see http://sphinx-doc.org/theming.html#builtin-themes). Other themes support such a change. What we are trying to do with the positioning of the sidebar on the left is hacking around the limitation of the current theme.

I see a few solutions here, that will help us avoid spending ages on a convoluted CSS file:

  • change the theme from sphinxdoc to something more flexible (see the link, there are a few to pick from),
  • hardcode the width of the sidebar to 300px and hope that the TOC in the sidebar won't have words overflowing (like in the LDAP example); relying on user screen resolution statistics (e.g. http://www.w3schools.com/browsers/browsers_display.asp) a resolution of the viewport in the range of 320x240px is rather unlikely,
  • go back to the initial floating sidebar (i.e. revert Fix overflow issues with plonematch.css. #12 and close this PR).

I'm happy with any option.

@mtbc
Copy link
Member

mtbc commented Jul 11, 2014

Floating seemed good to me, to be honest, but let's see how this looks.

@sbesson
Copy link
Member

sbesson commented Jul 11, 2014

My 2p about @bpindelski's comment:

  • solution 1 seem unreasonable in the context of 5.0.3 release and more generally of 5.0.x. If we want to go towards this route, I would be inclined to doing so on the develop branch for 5.1.0,
  • no strong preference between solutions 2 and 3,
  • arguably, the problem raised by this issue could also be solved at the content level. In the example above, should we not fix the chaindePasswordProvider* subheadings to have something more readable and adapted to the sidebar?

@bpindelski
Copy link
Author

@sbesson I can open a PR that changes the headings to shorter words. In such a case, do we want to revert to a floating layout?

@sbesson
Copy link
Member

sbesson commented Jul 14, 2014

To support Blazej's proposal, below are screeshots of three typical pages from our documentation:

Also /cc'ing @qidane who was involved in the desgin of the CSS theme.

@ghost
Copy link

ghost commented Jul 14, 2014

Hard to tell from the screenshots alone since it doesn't show what happens when you make the window smaller (e.g. reduce width by 50%). I'd suggest that 300px hardcoding is bad; on high dpi displays that's less than 1 inch, so could be <10 characters wide. Some tablets are now >350dpi, so pixels as a length specifier are not portable.

@bpindelski
Copy link
Author

@rleigh-dundee, @sbesson, @pwalczysko - as a compromise I've created a branch (https://github.com/bpindelski/sphinx_theme/tree/overflow2), where the layout is floating, but the "Note" and "Warning" boxes don't escape the flow of the text. See below screenshot:
screen shot 2014-07-14 at 16 42 58

@pwalczysko
Copy link
Member

Option 3, with the compromise solution as described above in #13 (comment) seems like the best option to me.

@bpindelski
Copy link
Author

@manics and @joshmoore complained that the sidebar is too wide, so I'll now push the changes from https://github.com/bpindelski/sphinx_theme/tree/overflow2 to this PR.

@manics
Copy link
Member

manics commented Jul 15, 2014

A general comment: The font-size of the sidebar text looks to be larger than that of the main docs, and with it's new position on the left I think it's too prominent. One option would be to reduce the font-size, which might also help with some of the width/formatting issues.

@bpindelski
Copy link
Author

@manics I will not be changing anything but the layout of elements in this PR, sorry. Lets finish this first and then we can have subsequent PRs with other modifications. I any other case this will grow exponentially, as everyone will have a comment.

@qidane
Copy link
Contributor

qidane commented Jul 15, 2014

If changing font sizes be careful how you do it. If set to a fixed size it will mess up the layout for people like me who have their browser set to use larger fonts. A relative size change would work better.

@bpindelski
Copy link
Author

@sbesson I've done push -f. The two commits are the consensus version which @pwalczysko liked.

@sbesson
Copy link
Member

sbesson commented Jul 15, 2014

@manics
Copy link
Member

manics commented Jul 15, 2014

Looks much better, thanks.

@sbesson
Copy link
Member

sbesson commented Jul 15, 2014

Thanks, @rleigh-dundee @pwalczysko thoughts?

@pwalczysko
Copy link
Member

@sbesson
Copy link
Member

sbesson commented Jul 15, 2014

Assuming the last layout satisfied most of the people, merging this PR. New features/improvements can be carried out in separate ones. Thanks @bpindelski and all for the feedback.

sbesson added a commit that referenced this pull request Jul 15, 2014
Change the width of the sidebar.
@sbesson sbesson merged commit 20491e7 into ome:master Jul 15, 2014
@bpindelski bpindelski deleted the sidebar-width branch July 15, 2014 16:06
@joshmoore
Copy link
Member

@bpindelski
Copy link
Author

@joshmoore, @sbesson See #15.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants