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

Stat with illustration padding issue #363

Merged

Conversation

nimithshetty17
Copy link

Fixes MERATIVE-950

Description

The stat with illustration block was designed to only have an image below it. We would like to update this block to ensure there is the proper padding added above/below when an image is not present.

Test URLs

Copy link

aem-code-sync bot commented Dec 6, 2023

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

Copy link

aem-code-sync bot commented Dec 6, 2023

Page Scores Audits Google
/drafts/Keith/real-world-evidence-copy PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

@keith-kaplan have added the bottom padding. Let me know if this is ok.

Copy link
Collaborator

@keith-kaplan keith-kaplan left a comment

Choose a reason for hiding this comment

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

@nimithshetty17 can we also add a class or add an edit that will make remove the top padding. I think it's default at 160px.

@nimithshetty17
Copy link
Author

@keith-kaplan have added a block with stat-with-illustration(no-padding) to add an optional padding. Let me know if this is ok. Can be reviewed here:- https://main--merative2--nimithshetty17.hlx.page/block-library/blocks/stat-with-illustration

@keith-kaplan
Copy link
Collaborator

I do not see the no-padding on top working. Can you check?

image

@nimithshetty17
Copy link
Author

@keith-kaplan
Copy link
Collaborator

keith-kaplan commented Dec 7, 2023

@keith-kaplan
Copy link
Collaborator

@nimithshetty17 I just saw the Figma file. I think we need to make the number stat a gradient like the content intro stat block.

@sachinmesh
Copy link

sachinmesh commented Dec 8, 2023

The width of the stat component is wrong, it should be desktop is 360px, tablet 275px, mobile 270px @nimithshetty17

image

CC: @keith-kaplan @anabarcelona

Copy link

aem-code-sync bot commented Dec 20, 2023

Page Scores Audits Google
/drafts/Keith/real-world-evidence-copy PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@nimithshetty17
Copy link
Author

@sachinmesh please check now.
cc: @keith-kaplan

Copy link

@sachinmesh sachinmesh left a comment

Choose a reason for hiding this comment

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

@sachinmesh sachinmesh added the Ready to merge Label for the pull requests that are ready to merge label Dec 20, 2023
Copy link
Collaborator

@keith-kaplan keith-kaplan 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.

@keith-kaplan keith-kaplan merged commit dd50eb0 into hlxsites:main Dec 20, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge Label for the pull requests that are ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants