Skip to content

Update sdg img size and .sustainable-dev-goal padding#3844

Merged
jdingeman merged 3 commits intohackforla:gh-pagesfrom
SZwerling:update-sdg-3738
Feb 14, 2023
Merged

Update sdg img size and .sustainable-dev-goal padding#3844
jdingeman merged 3 commits intohackforla:gh-pagesfrom
SZwerling:update-sdg-3738

Conversation

@SZwerling
Copy link
Member

Fixes #3738

What changes did you make and why did you make them ?

-All changes within $bp-media-up media query.

  • .sdg-image height: 145 px
  • .sustainable-dev-goal padding: 20px
  • p element within .sustainable-dev-goal margin-bottom: 0

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

image

Visuals after changes are applied

![image](
screencapture-localhost-4000-citizen-engagement-2023-01-19-13_59_39 (1).pdf
)

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b SZwerling-update-sdg-3738 gh-pages
git pull https://github.com/SZwerling/website.git update-sdg-3738

@github-actions github-actions bot added 2 weeks inactive P-Feature: Citizen Engagement https://www.hackforla.org/citizen-engagement role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours Complexity: Medium labels Jan 19, 2023
@jdingeman jdingeman self-requested a review January 25, 2023 01:35
@jdingeman
Copy link
Member

ETA: EOD 1/24
Availability: 2hrs

Copy link
Member

@jdingeman jdingeman left a comment

Choose a reason for hiding this comment

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

Hello. Nice job on this. The SDG images are sized correctly and padding looks good.

However in reading the issue, I believe the padding between cards should also be increased. It looks like in the published website and your current layout have a margin of 14px between cards:

Current layout margin

image

image

It looks like the figma design is proposing a margin of 30px between each card:

Figma design margin

image

@SZwerling
Copy link
Member Author

Thank you. I'll fix that card padding.

@jdingeman
Copy link
Member

jdingeman commented Feb 9, 2023

Hi @SZwerling, just wanted to check in to see if you are still working on this issue and could provide a progress update? If we don't hear back by Monday, we are going to close this pull request.

@SZwerling
Copy link
Member Author

Hello, @jdingeman. Apologies for delayed response.
I have updated the .program-area in tablet-up to have a margin top and bottom of 30px.
I pushed that to my remote repo. I am unsure what to do next. Another pull request on the same issue?
I am a little confused.
Thank you.

@SZwerling SZwerling requested a review from jdingeman February 11, 2023 20:29
@jdingeman
Copy link
Member

Hey @SZwerling , sorry about that, I see that you had pushed the commits prior to my comment. I will rereview.

@Jaretzbalba Jaretzbalba self-requested a review February 13, 2023 17:39
@Jaretzbalba
Copy link
Member

ETA: EOD 2/13
Availability: 2hrs

Copy link
Member

@Jaretzbalba Jaretzbalba left a comment

Choose a reason for hiding this comment

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

@SZwerling I see the changes to the padding and margin on the $bp-tablet-up media query reflect the new figma design. Everything looks to be uniform now and spaced as requested. Thank you for working on this issue!

Copy link
Member

@jdingeman jdingeman left a comment

Choose a reason for hiding this comment

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

Looks great @SZwerling !

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

Labels

Complexity: Medium P-Feature: Citizen Engagement https://www.hackforla.org/citizen-engagement role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Citizen Engagement (desktop): Update padding/margins and SDG image size

4 participants