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

Card style updated, created a single shared component [Fixes #2251] #2481

Merged
merged 7 commits into from
Mar 5, 2021

Conversation

Anuragtech02
Copy link
Contributor

Description

Created a single shared component CardItem in SharedStyledComponents and replaced the cards on /languages and /about ( #2076 & #2195 ) with the new shared component. Doing this also unified card hover transition.

Related Issue #2251

These card styles were outdated:
https://ethereum.org/en/contributing/translation-program/#in-progress so @samajammin I updated them to similar ones like on /languages and /about.

@netlify
Copy link

netlify bot commented Feb 24, 2021

Deploy request for ethereumorg rejected.

Rejected with commit 16789b3

https://docs.netlify.com/configure-builds/environment-variables/#sensitive-variable-policy

@github-actions github-actions bot added Status: Review Needed content 🖋️ This involves copy additions or edits labels Feb 24, 2021
Copy link
Contributor

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! A couple comments. Please let me know if you have any questions 😄

src/components/SharedStyledComponents.js Outdated Show resolved Hide resolved
src/components/SharedStyledComponents.js Outdated Show resolved Hide resolved
src/components/SharedStyledComponents.js Outdated Show resolved Hide resolved
src/components/Roadmap.js Outdated Show resolved Hide resolved
src/components/SharedStyledComponents.js Outdated Show resolved Hide resolved
@Anuragtech02
Copy link
Contributor Author

Thanks @samajammin !, I've made the changes

Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

@Anuragtech02 Thanks again for the PR =) Spotted two things... one is the redundant external arrow, the other being the bottom row expanding full width (see inline comments). Otherwise should be good to go!

src/components/TranslationsInProgress.js Show resolved Hide resolved
src/components/SharedStyledComponents.js Outdated Show resolved Hide resolved
@Anuragtech02
Copy link
Contributor Author

Thanks @wackerow ! I've added the hideArrow attr and set the flex grow to zero. I've also removed a redundant media query which was trying to set card width to 100% (which was already 100%).

Copy link
Contributor

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Almost there @Anuragtech02!

Looking at your preview deploy (https://deploy-preview-2481--ethereumorg.netlify.app/en/languages/) in mobile:
Image 2021-03-03 at 2 29 39 PM

The cards are no longer full width, like they are in production:
Image 2021-03-03 at 2 30 15 PM

I suspect the media query or the flex grow you deleted is actually needed on the language page items.

@Anuragtech02
Copy link
Contributor Author

Almost there @Anuragtech02!

Looking at your preview deploy (https://deploy-preview-2481--ethereumorg.netlify.app/en/languages/) in mobile:
Image 2021-03-03 at 2 29 39 PM

The cards are no longer full width, like they are in production:
Image 2021-03-03 at 2 30 15 PM

I suspect the media query or the flex grow you deleted is actually needed on the language page items.

@samajammin The media query was not the issue, it was the flex-grow. I added a media query for mobile screens which sets flex grow to 1 which now solves the issue for us.

Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

👍 @Anuragtech02 Looks good! Added one last little tweak to remove to right margin on mobile screens to even it out. Thanks again!
cc @samajammin

Copy link
Contributor

@samajammin samajammin left a comment

Choose a reason for hiding this comment

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

Thanks for following through on this ❤️

@samajammin samajammin merged commit 65bd8db into ethereum:dev Mar 5, 2021
@samajammin
Copy link
Contributor

@all-contributors please add @Anuragtech02 for code.

@allcontributors
Copy link
Contributor

@samajammin

I've put up a pull request to add @Anuragtech02! 🎉

@samajammin samajammin mentioned this pull request Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants