✨ Edit on GitHub link for Markdown files

TryGhost / docs

Ghost's official documentation

https://docs.ghost.org

MIT License

112 stars 80 forks source link

✨ Edit on GitHub link for Markdown files #86

Closed aileen closed 5 years ago

aileen commented 5 years ago

closes #62

💄

Added pencil icon and edit link
Updated styles
Show GitHub link only on large viewports

🤖

Created GitHub links from absolute file paths
Added fileAbsolutePath to Markdown fragments in order to read the absolute file path of the current page
Added a getGitHubLink to the Markdown post template which takes an absolute file path of the current page and tests it against our three known repositories/branches where we hold content.

netlify[bot] commented 5 years ago

Deploy preview for tryghost ready!

Built with commit 66e5feeb93ba97d87b0f566f1d1b2974e02ff7fe

https://deploy-preview-86--tryghost.netlify.com

JohnONolan commented 5 years ago

[edit: didn't realise it was WIP - email notification threw me off]

Have updated styles, those should be good to go now.

aileen commented 5 years ago

It's this automatic review assignment :(

aileen commented 5 years ago

@JohnONolan This is ready to go from my side, but the edit link is not responsive when the title is a bit longer. See here for example: https://deploy-preview-86--tryghost.netlify.com/api/handlebars-themes/

Shall I wrap it then? Or even not show it?

JohnONolan commented 5 years ago

It's this automatic review assignment :(

Maybe it won't do them w/ new draft PRs: https://github.blog/2019-02-14-introducing-draft-pull-requests/

Shall I wrap it then? Or even not show it?

Just hide it on smaller viewports - nobody is going to be editing on github from anything other than full desktop device anyway

aileen commented 5 years ago

@JohnONolan This is ready to merge, if you agree.

I disabled the GitHub link for anything that is not a large viewport. I went through many examples and this appears to be the right setting to hide it. It would still look ok on an iPad Pro, but normal iPad not. This doesn't seem to be differentiated w/ Tachyons, so I think we have two options here:

Leave it as is
Specify the breakpoint in a custom class instead of using Tachyons

(or option 3, which is probably not an option anyway: Show it on medium sized screens as well and only hide it on small screens)

JohnONolan commented 5 years ago

Nice! All looks good to me 🚀