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

fix(www): fix background on gatsby version indicator in showcase #20689

Merged
merged 2 commits into from
Jan 20, 2020

Conversation

keevan
Copy link
Contributor

@keevan keevan commented Jan 18, 2020

Resolves #20686

Also ensures the version and stars are always pinned to the top most
corner instead of sometimes floating based on the length of the author's
name

Description

Documentation

Related Issues

Resolves gatsbyjs#20686

Also ensures the version and stars are always pinned to the top most
corner instead of sometimes floating based on the length of the author's
name
@keevan keevan requested a review from a team as a code owner January 18, 2020 01:41
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Hey,

Thanks for creating this fix and finding this one. It hasn't crossed my mind yet so 👍. The fix your created works but is in my opinion on the right one.

Can you remove your change and edit
https://github.com/gatsbyjs/gatsby/pull/20689/files#diff-00f21c292fd295cc7e493fd0630f3c07R92
and add the following to the css here

-css={{ display: `flex`, justifyContent: `space-between` }}
+css={{ display: `flex`, justifyContent: `space-between`, align-items: `start` }}

@@ -102,7 +102,7 @@ const StartersList = ({ urlState, starters, count, sortRecent }) => {
>
{owner} /
</span>
<span css={{ display: `flex` }}>
<span css={{ display: `flex`, maxHeight: '1rem' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this one as it's more a hack than a fix.

Suggested change
<span css={{ display: `flex`, maxHeight: '1rem' }}>
<span css={{ display: `flex` }}>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for taking the time to review and suggest edits for my commit. Yes your proposed changes is definitely less hacky.

Using flex rules instead of maxHeight. Also updated to use backticks which I hadn't realised to maintain consistency.
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

awesome! thanks a bunch! This is great! 🎉

@wardpeet wardpeet changed the title Added a max height to fix the gatsby version indicator fix(www): fix background on gatsby version indicator in showcase Jan 20, 2020
@wardpeet wardpeet merged commit 6f8e716 into gatsbyjs:master Jan 20, 2020
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.

Gatsby starters version indicator is the wrong size on some starters
2 participants