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

Make tall code blocks scrollable #1146

Merged
merged 19 commits into from
Mar 8, 2019
Merged

Make tall code blocks scrollable #1146

merged 19 commits into from
Mar 8, 2019

Conversation

rooneyg21
Copy link
Contributor

closes #1123

@sindresorhus
Copy link
Member

This looks good!

It's smart to cut off in the middle of a line, but what if it's cut off on an empty line? Then it's not clear that there's more content. I think we should make it more obvious then. How about a slight bottom inset shadow to indicate there's more content? @bfred-it Thoughts?

source/content.css Outdated Show resolved Hide resolved
@fregante
Copy link
Member

@sindresorhus it’s hard to do that just with CSS because there’s no way to say “show only when there’s overflow.” It’d need some css trick

@fregante
Copy link
Member

This should probably be merged with the Gist max-height just to have them both in the same place and have both of them with the "scroll shadow" that Sindre suggested.

@rooneyg21
Copy link
Contributor Author

Haven't had a chance to check this out in a while. Adding a shadow would make it very clear that there is more content there if they scroll. Will look into it this weekend 👨‍💻

@rooneyg21
Copy link
Contributor Author

Hey Guys, finally got a chance to work on this. Here is how it looks when there is an overflow and when there isn't. Let me know what you guys think about the size of the shadow
screen shot 2018-03-27 at 2 34 07 am
screen shot 2018-03-27 at 2 52 17 am

I have also changed the selector to the following in order to limit this to only comments.

.comment-body > .highlight > blockquote,
.comment-body > .highlight > pre {
}

source/content.css Outdated Show resolved Hide resolved
source/content.css Outdated Show resolved Hide resolved
source/content.css Outdated Show resolved Hide resolved
@fregante
Copy link
Member

fregante commented Mar 29, 2018

Tests:

short
block
quote

short
code

long
long
long
long
long
long
long
long
long
long
long
long

nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
nested
long
long
long
long
long
long

nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code
nested code

long
long
long
long
long
long
long
long
long
long
long
quote

long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
long
code

 985:30  ✖  Expected newline after "," in a      value-list-comma-newline-after
            multi-line list
@fregante
Copy link
Member

fregante commented Mar 29, 2018

Screenshot

@fregante
Copy link
Member

@sindresorhus the only problem with those solutions is that they are background-based, so any image/pre in the blockquote will cover the shadow. Good enough?

@sindresorhus
Copy link
Member

@bfred-it Let's just use JS then, to make it appear.

source/content.css Outdated Show resolved Hide resolved
@fregante
Copy link
Member

Related: dear-github/dear-github#166

@fregante
Copy link
Member

Closing this PR since the suggested solution is completely different

@fregante
Copy link
Member

We're overthinking it. GitHub's own code embeds don't show any shadows. Same for StackOverflow (example)

We should drop the shadow and merge this

@fregante fregante reopened this Feb 16, 2019
@sindresorhus
Copy link
Member

Looks good, but needs to be mentioned in the readme. (Mostly so that the GitHub Papercuts team can more easily borrow the feature).

@lukehefson
Copy link

Looks good, but needs to be mentioned in the readme. (Mostly so that the GitHub Papercuts team can more easily borrow the feature).

lol so true

@fregante
Copy link
Member

Looks good, but needs to be mentioned in the readme

@arungalva are you still available to make these final changes to merge your feature? :)

@fregante fregante merged commit 5dba7e0 into refined-github:master Mar 8, 2019
@kanitw
Copy link

kanitw commented Mar 10, 2019

Given this is done directly in source/content.css, how can I disable this?

@kanitw
Copy link

kanitw commented Mar 10, 2019

NVM, just found I can override the CSS.

@dwelle
Copy link

dwelle commented Mar 23, 2019

For people's reference → to disable this feature, add this to the extension's custom css:

.comment-body blockquote,
.comment-body pre {
  max-height: none !important;
  overflow-y: visible !important;
}

image

@fregante fregante mentioned this pull request Mar 30, 2019
@chrisblossom
Copy link

@dwelle Is there an updated way to disable this? The custom css override is not working for me on firefox. Thanks!

@fregante
Copy link
Member

fregante commented Jul 8, 2019

@dwelle Is there an updated way to disable this? The custom css override is not working for me on firefox. Thanks!

It works if you add !important

I updated the example above

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

Successfully merging this pull request may close these issues.

Make tall code blocks scrollable
8 participants