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

Code appears to come out of the box #826

Closed
JC-ProgJava opened this issue Dec 9, 2020 · 6 comments
Closed

Code appears to come out of the box #826

JC-ProgJava opened this issue Dec 9, 2020 · 6 comments
Assignees
Milestone

Comments

@JC-ProgJava
Copy link
Contributor

JC-ProgJava commented Dec 9, 2020

Code in Ray Tracing in One Weekend seems to be appearing out of the box(see screenshot below).
A way you could fix this is to replace
overflow: visible;
with
overflow-x: scroll;
in

.md pre.listing.tilde {
    width: 96%;
    border-width: 3px;
    border-style: solid;
    border-color: rgb(212, 212, 212);
    border-image: initial;
    padding: 1.5ex;
    overflow: scroll;
    background: rgb(228, 228, 224);
}

in book.css.


Screenshot

@hollasch
Copy link
Collaborator

hollasch commented Dec 9, 2020

Sadly, this means that every code listing will be rendered with scrollbars, even though they've been designed to fit all code (with 96-character line lengths) on most browsers on standard displays (Chrome, Brave, Firefox, Edge).

We can improve this a bit, by only specifying horizontal scrollbars with overflow-x: scroll. (Also note that we definitely want overflow: visible for the media print styling, or useless scrollbar widgets render on printed copies.)

Are you browsing on a phone? If so, then yes, some lines will be too long. As always, there will be devices that are too narrow to fit the content, and the code boxes can't reflow. It was a trade-off we decided to accept, as we're mostly targeting printed copies or copies rendered on a full display (usually along with the coding environment of choice). We opted to ensure that all code is visible (using overflow: visible), but it may not look as pretty as on a full display.

If you can find a way to suppress rendering of the scrollbar when it's not needed, or detect and respond to windows that lack sufficient width (so we could, for example, reduce the code font size), then that would be quite useful. Let us know if you find a way to do this.

@hollasch hollasch self-assigned this Dec 9, 2020
@hollasch hollasch added this to the Backlog milestone Dec 9, 2020
@hollasch
Copy link
Collaborator

hollasch commented Dec 9, 2020

Just played around with this a bit more. I can simulate this by changing the zoom of the page so it's rendered very large.

It turns out that overflow-x: auto will display a horizontal scrollbar only if the content width exceeds that of the container. We can suppress scrollbars for printed media.

But sadly, this brings about another problem. With overflow: visible, the code will flow to full width, effectively widening the entire page. So you still have a scrollbar, but it's the page scrollbar. Code flows outside the code box, which isn't pretty, but allows to you view all code.

If you use overflow-x: auto, then when the code width exceeds the width of the listing box, you get a horizontal scrollbar. This seems great, except that for listing longer than the window height, the horizontal scrollbar itself is not scrolled out of view. So you have to use the window vetical scrollbar to scroll down to the listing horizontal scrollbar, then scroll that, then window vertical scroll back up to the code line you were reading — a pretty poor experience. With overflow: visible, you can always use the window horizontal scrollbar to scroll back and forth, wherever you are on the page.

The best solution I can think of is to have the listing box use a minimum-width or something to display normally on regular displays, but automatically widen when needed to include the full width of the text content. Let me know if you find a way to do that.

@JC-ProgJava
Copy link
Contributor Author

I see, perhaps the best thing you could do would be to add a media tag for adjusting the font size in smaller screens with something like:

@media only screen and (max-width: 550px) {
    .md pre.listing.tilde {
      font-size: 20px;
  }
}

@hollasch
Copy link
Collaborator

hollasch commented Dec 9, 2020

Ah, that's a good idea. Would you care to cobble something up and submit a PR?

@JC-ProgJava
Copy link
Contributor Author

JC-ProgJava commented Dec 9, 2020

The issue is, I don't think a smaller font size is going to help when readers are on a small screen. I guess leaving it as it is now isn't that bad when you'd have to have a trade-off between aesthetics and accessibility or visibility. I figured out another way, using

width: max-content;

which keeps the box width the same as the width of the text. The problem also seems to happen when I use my laptop, so this seems to improve the display on both types of devices. People using smaller screens may need to use the scrollbar on the page to see the entire box, is this fine? If it is, I'll submit a PR.

@hollasch
Copy link
Collaborator

hollasch commented Dec 9, 2020

Sounds good -- toss it up and I'll check it out.

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

No branches or pull requests

2 participants