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

turn of height restriction with scrolling by default #859

Closed
garretwilson opened this issue Jan 19, 2016 · 22 comments
Closed

turn of height restriction with scrolling by default #859

garretwilson opened this issue Jan 19, 2016 · 22 comments

Comments

@garretwilson
Copy link

I turned Prism on for my code samples and it looks gorgeous!

But a longer code section suddenly got its height restricted with scrollbars added with the coy skin. I didn't ask for this! I don't want scrollbars.

The reason this is a blocker issue is that I need to convert this documentation to a PDF. Scrollbars in a PDF won't help my students, and they will be missing code.

At the very least, there should be CSS that indicates the restricted height with scrollbars is only for screen media, not for print media!!

@garretwilson garretwilson changed the title turn of scrolling by default turn of height restriction with scrolling by default Jan 19, 2016
@garretwilson
Copy link
Author

The offending code is a max-height set for <pre>. I don't know if it is part of Prism or part of the coy skin.

Here is the workaround for CSS after Prism is included:

pre {
  max-height: none !important; /*workaround for Prism scrollbars*/
}

@zeitgeist87
Copy link
Collaborator

I don't know if it is part of Prism or part of the coy skin.

I think it is only part of the coy theme and I agree that such a restriction should not be determined in the theme but left to the end user. Changing it now would probably break a lot of websites which expect this behavior.

@garretwilson
Copy link
Author

I hate this "we can never ever change this for backwards-compatibility, even though it sucks" argument that keeps coming up.

Do it the right way: turn it off for print. Nobody could every want it for print, because you lose information. Leave it on temporarily for backwards-compatibility on web sites.

As for long-term, deprecate this behavior. Create a checkbox on the download page that you have to check to get the restricted height. Indicate that you will eventually remove it.

I shouldn't have to add workarounds in my clean code just to get things to work the right way.

@zeitgeist87
Copy link
Collaborator

As for long-term, deprecate this behavior. Create a checkbox on the download page that you have to check to get the restricted height. Indicate that you will eventually remove it.

I don't think there is any need for that. I think we can remove it immediately for the next release. If people want the old behavior, they can use an earlier version.

@garretwilson
Copy link
Author

Yay!! Thank you. Sorry, I guess I had misunderstood you earlier, thinking you were going to always leave it in.

@zeitgeist87
Copy link
Collaborator

@LeaVerou maybe you would like to comment on this? Otherwise I would remove the max-height: 30em attribute from the coy theme.

@LeaVerou
Copy link
Member

I would put it inside @media screen {...}. @garretwilson is right that nobody would want this for print.

@zeitgeist87
Copy link
Collaborator

I would put it inside @media screen {...}.

Done.

@garretwilson
Copy link
Author

Is this available from the website now? I just want to know when to test it. Thanks.

@garretwilson
Copy link
Author

P.S. I also recommend documenting somewhere how a user can change this or disable it altogether, so people don't have to go hunting around in a debugger. Just a thought. Thanks!

@zeitgeist87
Copy link
Collaborator

Is this available from the website now?

Yes. It is also in the gh-pages branch in this repo.

P.S. I also recommend documenting somewhere how a user can change this or disable it altogether, so people don't have to go hunting around in a debugger. Just a thought. Thanks!

I would be in favor of removing it altogether. It is inconsistent with the other themes.

@garretwilson
Copy link
Author

OK, the latest version from the site is printing without vertical scrollbars now. Thanks for addressing this so quickly!

I still get horizontal scrollbars for things I have made too wide, but I'm not sure what my opinion is on that. I suppose I just need to fix the content.

I would be in favor of removing it altogether. It is inconsistent with the other themes.

Fine with me!

@garretwilson
Copy link
Author

I would be in favor of removing it altogether. It is inconsistent with the other themes.

The problem with having this in by default is that there's no way to even guess what a "default" value might be for a majority of users. Best to leave it out altogether and make a note about what a user would need to do to restrict the height---although that should be pretty much self-explanatory, anyway.

@garretwilson
Copy link
Author

I would be in favor of removing it altogether. It is inconsistent with the other themes.

Yeah, I think that's the best option. Even after removing it from the print version, it still gives me trouble on a web page because I'm using it as a document in a screen-sharing session to give a class, and being forced to scroll two things (the larger document and then the code snippet) is a hassle. I'm going to go back to permanently removing it in my stylesheet, and I would recommend you get rid of it altogether too.

@LeaVerou
Copy link
Member

If we remove it, the theme breaks (the shadows are the wrong size). No time to debug why right now, but I suspect it has a lot to do with how hacky these shadows are. They've been giving us trouble with other things too (they break the line highlight plugin for example) but damn, they look nice :P

@garretwilson
Copy link
Author

That's odd, because I didn't notice a difference when I took the size restriction off (see workaround above).

Frankly the shadows aren't that great for me anyway. They look odd, as if the page is curled up. And when printing to a PDF from Chrome they look odd. And sometimes the shadow will show up half on one page and half on another. I'd be happy without the shadows in coy.

Coy is a pretty theme. No use trying to make it do too much. I'd be happy without the shadows.

@LeaVerou
Copy link
Member

I wouldn't be happy without the shadows and neither would many people who use it.
That said, putting them inside @media screen would be good to prevent them from showing up in print.

@zeitgeist87
Copy link
Collaborator

I wouldn't be happy without the shadows and neither would many people who use it.

I think I have fixed the shadow bug with #865.

@garretwilson
Copy link
Author

I wouldn't be happy without the shadows and neither would many people who use it.

That's fine, I can see how many people love it. It looks pretty on the screen (although it's a bit odd that I get it on both sides). Just let me know how to turn it off for print. Thanks.

@LeaVerou
Copy link
Member

We should put the shadows inside @media screen so they only show up on the screen.

@garretwilson
Copy link
Author

We should put the shadows inside @media screen so they only show up on the screen.

Do you plan on doing that soon? I don't want to rush you, but if it's going to take a while, do you know an easy way for me to turn them off from my own CSS? They are really starting to get in the way in my resulting PDFs. Thanks.

@garretwilson
Copy link
Author

do you know an easy way for me to turn them off from my own CSS?

So nobody knows how I can just turn off the shadows manually from my own CSS?

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

3 participants