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

Possible improvements for the "embed linked gists" feature #870

Closed
sindresorhus opened this issue Dec 12, 2017 · 12 comments
Closed

Possible improvements for the "embed linked gists" feature #870

sindresorhus opened this issue Dec 12, 2017 · 12 comments

Comments

@sindresorhus
Copy link
Member

sindresorhus commented Dec 12, 2017

#820 (comment)

  • some gists may be long or have multiple files
    GitHub limits their embeds to 12 lines + scroll: gh
  • there's no clear indicator of what file belongs to what gist like for the regular github.com auto-embeds:
  • there's no loading indicator, the content just pops into place at some point in whatever loading order
@scottnonnenberg
Copy link

scottnonnenberg commented Dec 13, 2017

In the Signal Desktop repository we frequently link to thousand-line gists, and they really don't work inline. I'd really appreciate a max lines limit for embedded files. For example: signalapp/Signal-Desktop#1884

(I've turned off Refined GitHub until something happens here)

@sindresorhus
Copy link
Member Author

@scottnonnenberg I agree. What do you think would be a sensible line limit?

@sindresorhus
Copy link
Member Author

sindresorhus commented Dec 13, 2017

Or should we just make it scrollable with a max height container?

@yakov116
Copy link
Member

What do you think would be a sensible line limit?

This

@scottnonnenberg
Copy link

What I want to avoid is completely losing the context of the discussion. A limit of something like half the height of the current viewport would help ensure that.

@fregante
Copy link
Member

fregante commented Dec 15, 2017

Like GitHub already does with permalink embeds, they should be limited to 12 lines via max-height + overflow: auto

The other problem is when they are multiple files.

  • limit to ~12 lines combined? 😕
  • avoid embedding?
  • group the files together? 👈
  • leave the original gist link as context?
  • embed them under a <detail> + <summary>? 👈
  • limit to one file per embed? 😕

@fregante
Copy link
Member

@scottnonnenberg this pr ⬆️ should let you re-enable Refined GitHub until a comprehensive solution is devised.

@sindresorhus
Copy link
Member Author

I think we should just avoid embedding if there are multiple files. The embedding feature is intended for simple gists.

@fregante
Copy link
Member

fregante commented Dec 15, 2017

Should we then show this info by the link? Otherwise one might think that the embed didn't work.

@sindresorhus
Copy link
Member Author

Can't we just document in the readme that it only applies to single-file gists?

@fregante
Copy link
Member

fregante commented Dec 15, 2017

Sure, but since we still have to load the gist and count the files we might as well attach some info, e.g.

https://gist.github.com/staltz/868e7e9bc2a7b8c1f754 (2 files)

@sindresorhus
Copy link
Member Author

Yeah, showing the file count could be useful.

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

No branches or pull requests

4 participants