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

MWPW-137282 - Action item block layout options #1501

Merged
merged 9 commits into from
Apr 29, 2024

Conversation

Sartxi
Copy link
Contributor

@Sartxi Sartxi commented Nov 7, 2023

The spacing and width of action items nested within the scroller and section grid are not currently correct for the action-icons 3-5 up design examples. This is due to the way the grid template items are styled in both places. So we are adding functionality to make the action icons design layouts possible.

  • Added .center style on section grid in desktop that will justify grid content center
  • Added default width on the action item to match designs and provide a function that applies item width conditional logic
  • Added center content style in action scroller for 5 or less items (also considers the established action item width logic)
  • Added Action scroller hide mask attribute to handle edge case I noticed, where mask was obscuring content when justified center in the action scroller
  • Removed unneeded grid style overrides in icon-block.css

Resolves: MWPW-137282

Test URLs:

@Sartxi Sartxi added new-feature New block or other feature needs-verification PR requires E2E testing by a reviewer labels Nov 7, 2023
Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@Sartxi Sartxi changed the title MWPW-137282 - Action item options for spacing within grid and scroller blocks MWPW-137282 - Action item options for spacing Nov 7, 2023
Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link

codecov bot commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.68%. Comparing base (7f196f5) to head (105f784).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1501      +/-   ##
==========================================
+ Coverage   96.65%   96.68%   +0.02%     
==========================================
  Files         165      165              
  Lines       43533    43522      -11     
==========================================
+ Hits        42077    42078       +1     
+ Misses       1456     1444      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

aem-code-sync bot commented Nov 7, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 8, 2023

Page Scores Audits Google
/drafts/sarchibeque/justify-action-item?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@Sartxi Sartxi changed the title MWPW-137282 - Action item options for spacing MWPW-137282 - Action item block layout options Nov 8, 2023
@Sartxi
Copy link
Contributor Author

Sartxi commented Nov 8, 2023

@ryanmparrish here are my other testing files in case they are helpful, thanks for reviewing!

All action block variants:
https://sarchibeque-mwpw-137282-center-actions--milo--adobecom.hlx.page/drafts/sarchibeque/actions-support-slider

Icon blocks using the center variant in section:
https://sarchibeque-mwpw-137282-center-actions--milo--adobecom.hlx.page/drafts/sarchibeque/icons-centered-grid

@Sartxi Sartxi marked this pull request as draft November 13, 2023 21:17
@Blainegunn Blainegunn changed the base branch from main to stage November 16, 2023 17:32
@Blainegunn Blainegunn changed the base branch from stage to main November 17, 2023 21:59
@Sartxi Sartxi closed this Nov 20, 2023
@Sartxi Sartxi deleted the sarchibeque/MWPW-137282-center-actions branch November 20, 2023 17:13
@Sartxi Sartxi restored the sarchibeque/MWPW-137282-center-actions branch April 16, 2024 22:31
@aem-code-sync aem-code-sync bot temporarily deployed to sarchibeque/MWPW-137282-center-actions April 16, 2024 22:31 Inactive
Copy link
Contributor

This PR is currently in the needs-verification state. Please assign a QA engineer to verify the changes.

Copy link
Contributor

@ryanmparrish ryanmparrish left a comment

Choose a reason for hiding this comment

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

@ryanmparrish
Copy link
Contributor

@Sartxi - One thing that I did notice, the tablet view doesn't use justify-items:center.
I see in the task that it states 'desktop' so it might be fine, but do we need to check w/ consonant or Figma on those? Generally when a section.center is used its a broad stroke across all viewports, so it might be worth a quick check.

Screen Shot 2024-04-25 at 10 40 48 AM

@elan-tbx elan-tbx added verified PR has been E2E tested by a reviewer and removed needs-verification PR requires E2E testing by a reviewer labels Apr 26, 2024
Ruchika4 and others added 2 commits April 29, 2024 17:04
* Update code owners for feds (#2194)

Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>

* Harden preflight link checking (#2169)

* Filter out empty `hrefs` before sending to spidy.
* Harden link check to be more robust against Spidy API.
* Update link check language.
* Better onboarding support.
  * Adds fallbacks if `.milo/config` has not been added.
* Fix for missing Word doc reference, under general tab.

Resolves: [MWPW-146695](https://jira.corp.adobe.com/browse/MWPW-146695)

Co-authored-by: Ryan Clayton <rclayton@adobe.com>

* MWPW-146755: RTL merch icon padding (#2162)

* RTL padding merch icon

* dep update

* MWPW-146001 parallelize literals call (#2187)

* MWPW-146001 parallelize literals call

* MWPW-146001 move promise to the function

* MWPW-146001 fix commerce library

* MWPW-146001 update literals endpoint

* MWPW-146001 fixing commerce ut

---------

Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>
@Blainegunn Blainegunn changed the base branch from main to stage April 29, 2024 16:34
@Blainegunn Blainegunn requested a review from a team as a code owner April 29, 2024 16:34
@Blainegunn Blainegunn merged commit b7e8900 into stage Apr 29, 2024
12 checks passed
@Blainegunn Blainegunn deleted the sarchibeque/MWPW-137282-center-actions branch April 29, 2024 16:34
mokimo pushed a commit to mokimo/milo that referenced this pull request Apr 30, 2024
* actions scroller center & section grid center

* update scroller test to cover all code branches

* update test mock to avoid nav test

* put back sticky and center styles

* [Release] Stage to Main (adobecom#2208)

* Update code owners for feds (adobecom#2194)

Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>

* Harden preflight link checking (adobecom#2169)

* Filter out empty `hrefs` before sending to spidy.
* Harden link check to be more robust against Spidy API.
* Update link check language.
* Better onboarding support.
  * Adds fallbacks if `.milo/config` has not been added.
* Fix for missing Word doc reference, under general tab.

Resolves: [MWPW-146695](https://jira.corp.adobe.com/browse/MWPW-146695)

Co-authored-by: Ryan Clayton <rclayton@adobe.com>

* MWPW-146755: RTL merch icon padding (adobecom#2162)

* RTL padding merch icon

* dep update

* MWPW-146001 parallelize literals call (adobecom#2187)

* MWPW-146001 parallelize literals call

* MWPW-146001 move promise to the function

* MWPW-146001 fix commerce library

* MWPW-146001 update literals endpoint

* MWPW-146001 fixing commerce ut

---------

Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>

---------

Co-authored-by: Ruchika Sinha <69535463+Ruchika4@users.noreply.github.com>
Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>
mokimo pushed a commit to mokimo/milo that referenced this pull request Apr 30, 2024
* actions scroller center & section grid center

* update scroller test to cover all code branches

* update test mock to avoid nav test

* put back sticky and center styles

* [Release] Stage to Main (adobecom#2208)

* Update code owners for feds (adobecom#2194)

Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>

* Harden preflight link checking (adobecom#2169)

* Filter out empty `hrefs` before sending to spidy.
* Harden link check to be more robust against Spidy API.
* Update link check language.
* Better onboarding support.
  * Adds fallbacks if `.milo/config` has not been added.
* Fix for missing Word doc reference, under general tab.

Resolves: [MWPW-146695](https://jira.corp.adobe.com/browse/MWPW-146695)

Co-authored-by: Ryan Clayton <rclayton@adobe.com>

* MWPW-146755: RTL merch icon padding (adobecom#2162)

* RTL padding merch icon

* dep update

* MWPW-146001 parallelize literals call (adobecom#2187)

* MWPW-146001 parallelize literals call

* MWPW-146001 move promise to the function

* MWPW-146001 fix commerce library

* MWPW-146001 update literals endpoint

* MWPW-146001 fixing commerce ut

---------

Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>

---------

Co-authored-by: Ruchika Sinha <69535463+Ruchika4@users.noreply.github.com>
Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>
mokimo pushed a commit to mokimo/milo that referenced this pull request Apr 30, 2024
* actions scroller center & section grid center

* update scroller test to cover all code branches

* update test mock to avoid nav test

* put back sticky and center styles

* [Release] Stage to Main (adobecom#2208)

* Update code owners for feds (adobecom#2194)

Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>

* Harden preflight link checking (adobecom#2169)

* Filter out empty `hrefs` before sending to spidy.
* Harden link check to be more robust against Spidy API.
* Update link check language.
* Better onboarding support.
  * Adds fallbacks if `.milo/config` has not been added.
* Fix for missing Word doc reference, under general tab.

Resolves: [MWPW-146695](https://jira.corp.adobe.com/browse/MWPW-146695)

Co-authored-by: Ryan Clayton <rclayton@adobe.com>

* MWPW-146755: RTL merch icon padding (adobecom#2162)

* RTL padding merch icon

* dep update

* MWPW-146001 parallelize literals call (adobecom#2187)

* MWPW-146001 parallelize literals call

* MWPW-146001 move promise to the function

* MWPW-146001 fix commerce library

* MWPW-146001 update literals endpoint

* MWPW-146001 fixing commerce ut

---------

Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>

---------

Co-authored-by: Ruchika Sinha <69535463+Ruchika4@users.noreply.github.com>
Co-authored-by: Narcis Radu <github@narcisradu.ro>
Co-authored-by: Blaine Gunn <Blainegunn@gmail.com>
Co-authored-by: Robert Bogos <146744221+robert-bogos@users.noreply.github.com>
Co-authored-by: Ryan Clayton <rgclayton@gmail.com>
Co-authored-by: Ryan Clayton <rclayton@adobe.com>
Co-authored-by: Axel Cureno Basurto <axelcureno@gmail.com>
Co-authored-by: Nicolas Peltier <1032754+npeltier@users.noreply.github.com>
@vhargrave vhargrave restored the sarchibeque/MWPW-137282-center-actions branch April 30, 2024 09:37
@aem-code-sync aem-code-sync bot temporarily deployed to sarchibeque/MWPW-137282-center-actions April 30, 2024 09:37 Inactive
@vhargrave vhargrave deleted the sarchibeque/MWPW-137282-center-actions branch April 30, 2024 09:41
@mokimo
Copy link
Contributor

mokimo commented Apr 30, 2024

@vhargrave
Copy link
Contributor

mokimo added a commit that referenced this pull request Apr 30, 2024
Revert "MWPW-137282 - Action item block layout options (#1501)"

This reverts commit b7e8900.
@mokimo
Copy link
Contributor

mokimo commented Apr 30, 2024

@Sartxi you will need to re-raise this PR once the issue is fixed

@mokimo mokimo deleted the sarchibeque/MWPW-137282-center-actions branch August 15, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature New block or other feature Ready for Stage verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants