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-131763] Variable Aside backgrounds based on media breakpoints #1162

Merged
merged 43 commits into from
Sep 21, 2023

Conversation

elan-tbx
Copy link
Contributor

@elan-tbx elan-tbx commented Aug 25, 2023

  • Recent updates to the Aside block did not allow for CSS values to be used in the multi-background stacking order. This PR addresses that.
  • The decorateBlockBg() function in decorate.js has been optimized/simplified, and now replaces the decorateBlockBg() definition that was within aside.js
  • The requisite CSS for handling variable backgrounds has been added to aside.css
  • Stylelinting errors have been fixed in aside.css

Resolves: MWPW-131763

Test URLs:

NOTE: Resize your browser window when viewing test URLs to test variable breakpoints.

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 25, 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 25, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 26, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@codecov
Copy link

codecov bot commented Aug 26, 2023

Codecov Report

Merging #1162 (ca5d01a) into main (db1af1a) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1162      +/-   ##
==========================================
+ Coverage   96.25%   96.27%   +0.02%     
==========================================
  Files         137      137              
  Lines       33298    33281      -17     
==========================================
- Hits        32050    32042       -8     
+ Misses       1248     1239       -9     
Files Changed Coverage Δ
libs/blocks/aside/aside.js 100.00% <100.00%> (ø)
libs/blocks/marquee/marquee.js 92.81% <100.00%> (ø)
libs/utils/decorate.js 100.00% <100.00%> (+5.48%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 26, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Aug 26, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@elan-tbx elan-tbx marked this pull request as ready for review August 26, 2023 00:26
@elan-tbx elan-tbx requested review from Sartxi and a team as code owners August 26, 2023 00:26
@skumar09 skumar09 added the run-nala Run Nala Test Automation against PR label Aug 28, 2023
@meganthecoder
Copy link
Contributor

meganthecoder commented Aug 29, 2023

Are you sure the decorateBlockBg export from decorate.js wasn't being used anywhere? That doesn't seem likely

Edit: I just checked and I see it in 4 different blocks. Can you check that your new code works with all of those blocks and provide examples for testing?

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 19, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 19, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 19, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 19, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 19, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 20, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 20, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@Sartxi Sartxi added verified PR has been E2E tested by a reviewer and removed needs-verification PR requires E2E testing by a reviewer labels Sep 20, 2023
@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Sep 21, 2023

Page Scores Audits Google
/drafts/ebartholomew/aside?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@honstar honstar merged commit d5bdb06 into main Sep 21, 2023
11 checks passed
@honstar honstar deleted the ebartholomew/aside-bg-breakpoints branch September 21, 2023 13:32
vgoodric pushed a commit to vgoodric/bootcamp-milo that referenced this pull request Feb 1, 2024
…dobecom#1162)

* add requisite styles for aside bg breakpoint switching

* clean up linting errors in decorate.js, simplify decorateBlockBg and better handle color backgrounds

* update global decorateBlockBg, use in aside

* autoBlock races with bg creation, defer to video

* fix stylelint errors

* update test markup to align w/ video auto-blocking

* add coverage for variable CSS backgrounds

* remove test src's, make bg decorator more readable

* make global base bg styles, remove dupes from aside.css

* add con-block bg styles to main styles file

* regroup breakpoints when only two bgs are provided

* reintroduce video decorator to support additional video types

* update breakpoint classnames

* revert previous commit

* remove bg media min height

---------

Co-authored-by: Honwai Wong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-nala Run Nala Test Automation against PR verified PR has been E2E tested by a reviewer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants