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

Fix code being wrapped incorrectly #33

Merged
merged 1 commit into from
Sep 2, 2016
Merged

Fix code being wrapped incorrectly #33

merged 1 commit into from
Sep 2, 2016

Conversation

crasm
Copy link
Contributor

@crasm crasm commented Jul 31, 2016

This should prevent code from being wrapped when displayed on the site. I'm not sure if it's just my font choices, but that ideally wouldn't affect the code wrapping. (Anyone else have this problem other than me?)

Screenshot of Before
Screenshot of After

I've tested this in chromium and firefox by editing the css in-browser, but I wasn't able to test the actual changes to the sass file.

This should prevent code from being wrapped when displayed on the site.
@rgson
Copy link

rgson commented Sep 1, 2016

Some critique, quoted from #31 (comment)

#33 may well fix it in many cases, though the new number also seems rather arbitrary/unreliable. Unless I'm mistaken, em is determined by the height of the font (and not its width). If that's the case then 60 em is still not guaranteed to be wide enough.

The ch unit could possibly be used to get some level of guarantee. It's supposed to be the actual width of the character 0. In a monospace font it should thus be the actual width of all characters. However, there seems to be some compatibility issues with IE (http://caniuse.com/#feat=ch-unit). It'd also have to be applied to the code section of the page instead of to .container in order for it to measure the correct font.

Another alternative would be Javascript; to programmatically measure the width of a character and change the width of the code section to hold 80 of them. A reasonable default would of course be kept in the CSS as fallback.

@adambard
Copy link
Owner

adambard commented Sep 2, 2016

LGTM, thanks!

@adambard adambard merged commit 717d41c into adambard:master Sep 2, 2016
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

Successfully merging this pull request may close these issues.

3 participants