Skip to content

Conversation

@GeoffreyBooth
Copy link
Collaborator

@GeoffreyBooth GeoffreyBooth commented Sep 19, 2017

  • Fix Website: Some changelog entries have very large text #4702: Some changelog entries are too large.
  • Fix Fix CoffeeScript.org's use of pushState #4704: Back/forward not behaving as expected. Now new browser history items (which are cycled through by back and forward) only happen for user clicks on links; opening and closing Try CoffeeScript doesn’t count as a new history entry, and closing it retrieves the previous URL. The URLs still change on scroll, but scrolling doesn’t add to the browser history.
  • Eliminate the 1px jitter when initializing code editors.
  • Use Bootstrap’s breakpoints for the only-slightly-larger text on larger screens.
  • Use Bootstrap’s font size percentage and padding styling for the <code> element, to avoid misaligned baseline.
  • Buttons that just run the code block shouldn’t have a label, since in all the other examples the label signifies the command to be run.
  • Mobile Safari needs the code to be >= 16px, or else it will zoom in on it no matter what we put in the <head>.

http://rawgit.com/GeoffreyBooth/coffeescript/docs-fixes/docs/v2/

Copy link
Collaborator

@lydell lydell left a comment

Choose a reason for hiding this comment

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

I don't know what to review here, really. You know the website best @GeoffreyBooth.

.main blockquote {
.main li p, .main li li, .main li blockquote {
font-size: 1em;
}
Copy link
Collaborator

@lydell lydell Sep 19, 2017

Choose a reason for hiding this comment

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

This feels more like a band-aid than a proper fix, but whatever. It works.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know. I feel dirty writing it. But I have an aversion to rem and px units, because long ago they were bad for accessibility, but probably aren’t anymore.

@GeoffreyBooth
Copy link
Collaborator Author

This PR is mostly to double-check I didn’t break anything, since several recent commits did. And to clear that you both are okay with these changes.

@jashkenas
Copy link
Owner

A few teeny adjustments:

  • Let's go from 1.5 to 1.7 on the body text line-height, to give it a little more breathing room.

  • Let's tweak the background padding on the code { rule to add:

    padding: 2px 8px;
    line-height: 90%;

... so that the boxes slim down a little.

  • Let's set a max-width of 1050px on the sections , to keep the text and code from stretching out awkwardly wide on big monitors.

  • On desktop-sized screens, for the code, let's go to font-size 14px, and line-height 20px (or em equivalents) to fit the code to the layout better. Let's unify that sizing across code, pre, and CodeMirror.

  • Not how to make the flexbox for it work, but it would be great if the sidebar had a max-width of around 380px, instead of continuing to expand on wide screens.

  • Our lists of resources and books need some margin. margin-bottom: 20px for the lis would do it.

@GeoffreyBooth
Copy link
Collaborator Author

@jashkenas I’ve applied your notes, with a few caveats (let’s discuss):

  • The code was already line-height: 90% per Bootstrap, which comes to 20px. As for padding: 2px 8px, that makes the inline code background shorter but also wider; is that your intent? Were you meaning to adjust just the inline code, or the code blocks, or both?

  • I agree we should constrain the main column of text, but I want to avoid seeing word wrapping code in our side-by-side code examples. How about we follow Bootstrap’s lead and limit the width of text, while letting the editable code blocks fill all available width? Bootstrap does a max-width: 80% which feels about right. Since it’s a percentage, I’m only applying it to Bootstrap’s largest breakpoint (> 1200px).

  • I spent many fruitless hours trying to get the sidebar to have a “natural” width, the width it would get from the widest line of text, and no wider. That’s easily achievable with flexbox—if you don’t use position: fixed. In other words, since the sidebar is outside of the flow of the main column, its flexboxness can’t “push against” the text of the main column, and an absolutely-positioned flexbox element naturally sizes to 100% wide. (It’s been a few weeks, so I’m sure I’m not explaining this correctly.) Long story short, I agree the sidebar is too damn wide on desktop, and it seems like slapping a max-width on there for large breakpoints should be an easy fix, but I dimly remember trying that and running into issues from that. So I left it as it is. Can we maybe punt that one to a future PR, so that it can get a more thorough review on its own? I know I ran into enough unexpected issues that it should get a more thorough check on various browsers, rather than messing with it quickly now.

  • I think all the list items could use some bottom margin, not just the ones in Resources. I gave them a little more than half the bottom margin that paragraphs get. I tried matching the paragraph bottom margin, but then they looked too spaced out IMO, especially in the changelog but also in the resources list (paragraphs get 21px). I gave a generous amount on the announcement page, though, but somehow I feel like it looks better there. Maybe because the lists are shorter.

http://rawgit.com/GeoffreyBooth/coffeescript/docs-fixes/docs/v2/

@jashkenas
Copy link
Owner

Looking good!

  • If we can't get the sidebar skinny on desktop for now, then yes, let's punt as a nice to have for later. I also tried with flexbox, and failed. I'm guessing a non-flex solution might work.

Otherwise, looks great to me. Please merge.

@GeoffreyBooth GeoffreyBooth merged commit 5cbd25f into jashkenas:master Sep 20, 2017
@GeoffreyBooth GeoffreyBooth deleted the docs-fixes branch September 20, 2017 20:10
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.

Fix CoffeeScript.org's use of pushState Website: Some changelog entries have very large text

3 participants