Skip to content

Edit on GitHub feature for content#949

Merged
wookie184 merged 8 commits into
mainfrom
hedyhli-edit-on-github
May 6, 2023
Merged

Edit on GitHub feature for content#949
wookie184 merged 8 commits into
mainfrom
hedyhli-edit-on-github

Conversation

@jchristgit
Copy link
Copy Markdown
Contributor

@jchristgit jchristgit commented Apr 21, 2023

Pulled from #626 to allow maintainer edits

Hidden in mobile:

image

Closes #459.

@jchristgit jchristgit self-assigned this Apr 21, 2023
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 21, 2023

Deploy Preview for pydis-static ready!

Name Link
🔨 Latest commit dc17b3d
🔍 Latest deploy log https://app.netlify.com/sites/pydis-static/deploys/64561856f5add500085185a7
😎 Deploy Preview https://deploy-preview-949--pydis-static.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 21, 2023

Coverage Status

Coverage: 100.0%. Remained the same when pulling dc17b3d on hedyhli-edit-on-github into 27966e2 on main.

@jchristgit jchristgit changed the title Edit on GitHub feature Edit on GitHub feature for content articles feature Apr 21, 2023
@jchristgit jchristgit requested review from kosayoda and ks129 April 21, 2023 19:40
@jchristgit jchristgit marked this pull request as ready for review April 21, 2023 19:40
@jchristgit jchristgit removed their assignment Apr 21, 2023
@jchristgit jchristgit changed the title Edit on GitHub feature for content articles feature Edit on GitHub feature for content Apr 21, 2023
@wookie184
Copy link
Copy Markdown
Contributor

Comparing https://deploy-preview-949--pydis-static.netlify.app/pages/guides/pydis-guides/contributing/sir-lancebot/ to https://www.pythondiscord.com/pages/guides/pydis-guides/contributing/sir-lancebot/ it seems like this causes the rest of the page to drop down a bit.

Before:
image

After:
image

Any ideas on why this is?

@jchristgit
Copy link
Copy Markdown
Contributor Author

@wookie184 yes I have seen that, but I have no idea what causes it. Inspecting the layout in Firefox I concluded that it's no margin or padding from the element itself. I also concluded that it's probably not noticeable by anybody and was hoping I could sweep it under the rug. Unfortunately, you have crossed through my plan to do it.

Any other idea where we can check what's causing the surplus whitespace apart from the layout box in devtools? I also tried to use a span instead of div to no success.

@ChrisLovering
Copy link
Copy Markdown
Member

ChrisLovering commented Apr 28, 2023

@jchristgit
Copy link
Copy Markdown
Contributor Author

I saw these but I don't really have the CSS-fu to drive it to the finish line with the vertical stacking :(

@hedyhli
Copy link
Copy Markdown
Contributor

hedyhli commented Apr 29, 2023

Hey there, so I've played around with the HTML a little and it seems this patch works:
https://paste.pythondiscord.com/raw/fexodipane

Basically comparing the elements from before this PR and after, the container appears to gained a height, and the breadcrumb <section> also changed because of the added flex div element. So I decided to take it out of the container, then modify the CSS classes a little.

Hopefully this diff isn't a 'hacky' fix though (otherwise it might pose problems in the future).

Note: If you compare the positioning of sub-articles and edit-on-gh before and after applying the patch, you might notice the two having moved towards the right a little, it could possibly be fixed with something like a margin-right. IMO it's more trivial than the entire content dropping down as wookie pointed out.

Hedy Li and others added 8 commits May 6, 2023 11:04
- Using `if pages` to check whether the page is an article or category
  doesn't work for /pages/guides
- I still need to modify the styling and position of the button a bit
- Should probably just use a static method and put src_url as context
  instead of having a template tag
- Before the title
- Aligned left - more mobile-friendly
- Simpler page structure
- I personally don't think it makes sense for a simple link to a page's
  source be a button. Plus, it's similar to the style of other docs like
  VS Code
@jchristgit jchristgit force-pushed the hedyhli-edit-on-github branch from 9053b55 to dc17b3d Compare May 6, 2023 09:05
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @hedyhli for the help with this!

@wookie184 wookie184 merged commit 748a170 into main May 6, 2023
@wookie184 wookie184 deleted the hedyhli-edit-on-github branch May 6, 2023 17:01
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.

Edit on GitHub for content articles.

5 participants