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.

Sign up for GitHub

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

Jump to bottom

23 card block #48

Merged
merged 16 commits into from
Sep 6, 2023
Merged

23 card block #48

merged 16 commits into from
Sep 6, 2023

Conversation

TomaszDziezykNetcentric
Copy link
Collaborator

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 17, 2023

Hello, I'm Franklin 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

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 18, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric TomaszDziezykNetcentric marked this pull request as ready for review August 18, 2023 09:41
Lakshmishri
Lakshmishri requested changes Aug 18, 2023
View reviewed changes
Copy link
Collaborator

@Lakshmishri Lakshmishri left a comment

Choose a reason for hiding this comment

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

Can you change these in style.css

--headline-1-line-letter-spacing to --headline-1-letter-spacing

--headline-2-line-letter-spacing to --headline-2-letter-spacing
Screenshot 2023-08-18 at 15 23 18 the arrow and text don't look aligned

kesiah
kesiah requested changes Aug 21, 2023
View reviewed changes
Copy link
Collaborator

@kesiah kesiah left a comment

Choose a reason for hiding this comment

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

  • I see a typo in the code, dark-backgorund background is not correctly written. Also not sure if it's possible we rename the class to section--dark
  • Missing hover state of the button
  • In tablet, I agree on wrapping the items but not centering them, we can ask the agency what to do here. I would suggest that every item should take 50% of the content and justify it to the left.
image

blocks/v2-cards/v2-cards.js Outdated Show resolved Hide resolved
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 28, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric
Copy link
Collaborator Author

Can you change these in style.css

--headline-1-line-letter-spacing to --headline-1-letter-spacing

--headline-2-line-letter-spacing to --headline-2-letter-spacing

Screenshot 2023-08-18 at 15 23 18 the arrow and text don't look aligned

Fixed

@TomaszDziezykNetcentric
Copy link
Collaborator Author

  • I see a typo in the code, dark-backgorund background is not correctly written. Also not sure if it's possible we rename the class to section--dark

    • Missing hover state of the button

    • In tablet, I agree on wrapping the items but not centering them, we can ask the agency what to do here. I would suggest that every item should take 50% of the content and justify it to the left.

image

1st and 2nd point fidex. 3rd is implemented as described in the issue :) - @kesiah

kesiah
kesiah requested changes Aug 30, 2023
View reviewed changes
Copy link
Collaborator

@kesiah kesiah left a comment

Choose a reason for hiding this comment

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

dark-background is not working :( I imagine we need to change the classname in code.

styles/styles.css Outdated Show resolved Hide resolved
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 30, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

cogniSyb
cogniSyb requested changes Aug 30, 2023
View reviewed changes
styles/styles.css Show resolved Hide resolved
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 31, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 31, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 5, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 5, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

kesiah
kesiah requested changes Sep 5, 2023
View reviewed changes
blocks/v2-cards/v2-cards.css Outdated Show resolved Hide resolved
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 5, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

cogniSyb
cogniSyb requested changes Sep 5, 2023
View reviewed changes
blocks/v2-cards/v2-cards.js Outdated Show resolved Hide resolved
styles/styles.css Outdated Show resolved Hide resolved
styles/styles.css Outdated Show resolved Hide resolved
icons/speach.svg Outdated Show resolved Hide resolved
icons/play.svg Outdated Show resolved Hide resolved
icons/chevron-right.svg Outdated Show resolved Hide resolved
blocks/v2-cards/v2-cards.css Outdated Show resolved Hide resolved
blocks/v2-cards/v2-cards.css Outdated Show resolved Hide resolved
blocks/v2-cards/v2-cards.css Outdated Show resolved Hide resolved
blocks/v2-cards/v2-cards.css Outdated Show resolved Hide resolved
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 5, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 5, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/ PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@Lakshmishri Lakshmishri merged commit 206675f into main Sep 6, 2023
2 checks passed
@Lakshmishri Lakshmishri deleted the 23-card-block branch September 6, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kesiah kesiah kesiah approved these changes

@Lakshmishri Lakshmishri Lakshmishri approved these changes

@cogniSyb cogniSyb cogniSyb approved these changes

@SantiagoHomps-NC SantiagoHomps-NC Awaiting requested review from SantiagoHomps-NC

@avinashNChavan avinashNChavan Awaiting requested review from avinashNChavan

Assignees

@TomaszDziezykNetcentric TomaszDziezykNetcentric

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cards
5 participants
@TomaszDziezykNetcentric @kesiah @Lakshmishri @cogniSyb @tdziezykDS