-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Increase page max width to show more content when possible #5523
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
base: rolling
Are you sure you want to change the base?
Increase page max width to show more content when possible #5523
Conversation
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
HTML artifacts: https://github.com/ros2/ros2_documentation/actions/runs/14839938935/artifacts/3063058012. To view the resulting site:
|
source/_static/custom.css
Outdated
@@ -0,0 +1,3 @@ | |||
.wy-nav-content { | |||
max-width: 1000px !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the !important
strictly necessary or can we find a way to do this with correct CSS precedence? Using !important
competes with userstyles that are doing exactly similar things in user stylesheets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used/kept it because of this comment, which does mention that styles may not all play nice depending on the extension: readthedocs/sphinx_rtd_theme#295 (comment). I removed it and quickly looked through some pages, and I don't see anything wrong, so let's do that. We can always reevaluate later if we find actual issues.
source/_static/custom.css
Outdated
@@ -0,0 +1,3 @@ | |||
.wy-nav-content { | |||
max-width: 1000px !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
max-width: 1000px !important; | |
max-width: 64rem !important; |
Using rem
instead of pixel values will scale the viewport with font-size. So if someone increases the font size the viewport will expand to accommodate it. 64rem * 16pt = 1024px.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm no CSS expert, so thanks for the info! Proper scaling was also discussed in readthedocs/sphinx_rtd_theme#295. That makes sense.
One could say, it's not very... important. Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this increases responsiveness on devices with varying screen resolutions and user settings for font size, and flexibility making the layout more scalable to changes in user preferences or browser zoom levels.
i am not CSS expert... maybe we can merge this with @nuclearsandwich 's approval?
I was honestly expecting more reticence about the actual width increase part:
but I'm happy if people think this is a good idea. |
One question: Should we only do this with the Rolling docs for now? Then, if we haven't found any issues after some time, we can backport this to other branches. |
This aims to increase the page's maximum width so that it can show more content when possible.
From what I could tell, the current max width is 800px. It's a bit funny to have to scroll horizontally for somewhat-long commands when there's plenty of space, e.g., on an ultrawide 3840×1600 monitor:
This PR adds a custom CSS file that sets the max width to 1000px. The page still correctly adapts if the width is less than that, and this allows us to see more content without having to scroll horizontally.
I found this relevant issue/discussion: readthedocs/sphinx_rtd_theme#295. There are some valid points against increasing the max width. One of them is that a wider page might decrease reading comprehension: readthedocs/sphinx_rtd_theme#295 (comment). I think this is true, but increasing the width from 800px to 1000px might not be that bad and could help a lot.
It could break things that rely on a specific width, but I've taken a quick look and don't see anything obviously broken.
I opened this as a prototype so that we could discuss it. We could make it bigger if we want, e.g., 1200px, but that might be too much. I thought this might've been discussed before (I sort of vaguely remember something about increasing the page width), but I couldn't find any issue here.