MWPW-109331 - Carousel #375

rgclayton · 2023-01-07T00:31:10Z

Milo Carousel

Supports multiple carousels on a page
Default displays one slide at a time
Multi slide variants to show 2-6 slides at a time via block variants (show-2, show-3, show-4, show-5, show-6)
- Multi view slides advance one slide at a time
Container variant (confines carousel to 1200px)
Full width carousel. No variant needed
Next, previous button navigation
Keyboard navigation
Slide indicators that indicate the number of slides in the carousel for single slide variation. Indicators are clickable to jump to specific slide (non-linear navigation)
Swipe navigation for touch enabled devices

Test URLs:

BACOM Test URLs:

aem-code-sync · 2023-01-07T00:31:12Z

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 · 2023-01-07T00:31:34Z

Page	Score	PSI Audit	Google
/drafts/rclayton/carousel-demo?martech=off

codecov · 2023-01-07T00:32:39Z

Codecov Report

Merging #375 (db0b925) into main (f45b234) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #375      +/-   ##
==========================================
+ Coverage   98.91%   98.92%   +0.01%     
==========================================
  Files          78       78              
  Lines       20367    20745     +378     
==========================================
+ Hits        20145    20523     +378     
  Misses        222      222

Impacted Files	Coverage Δ
libs/blocks/carousel/carousel.js	`100.00% <100.00%> (ø)`
libs/utils/utils.js	`97.52% <100.00%> (+0.01%)`	⬆️

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

chrischrischris · 2023-01-09T16:48:28Z

libs/blocks/carousel/carousel.js

+  btnsArray.push(previousBtn, nextBtn);
+  return btnsArray;


nit:

Suggested change

btnsArray.push(previousBtn, nextBtn);

return btnsArray;

return [previousBtn, nextBtn];

chrischrischris · 2023-01-09T16:55:50Z

libs/blocks/carousel/carousel.js

+
+  el.append(...nextPreviousBtns, controlsContainer);
+
+  parentArea.addEventListener('milo:deferred', () => {


Should the event listener be removed after milo:deferred has fired?

That event only fires once per page.

cc: @auniverseaway

may as well remove the event listener in that case

aem-code-sync · 2023-01-10T20:48:20Z

Page	Score	PSI Audit	Google
/drafts/rclayton/carousel-demo?martech=off

aem-code-sync · 2023-01-11T22:50:56Z

Page	Score	PSI Audit	Google
/drafts/rclayton/carousel-demo?martech=off

vhargrave

Really nice job - the carousel is super robust in my opinion.
I found a few small nitpicked things and I saw the unit tests are failing, but after that it should be good.

libs/blocks/carousel/carousel.js

vhargrave · 2023-01-12T12:34:27Z

libs/blocks/carousel/carousel.js

+  parentArea.addEventListener('milo:deferred', () => {
+    const images = el.querySelectorAll('img[loading="lazy"]');
+    images.forEach((img) => {
+      img.removeAttribute('loading');


why remove the loading lazy from the images?

It was causing a flicker on images inside of the Carousel when you advanced to a slide that contained images. Due to them not loading until they came in to view.

vhargrave · 2023-01-12T12:35:21Z

libs/utils/utils.js

@@ -509,6 +511,8 @@ async function loadPostLCP(config) {
 }

 export async function loadDeferred(area, blocks, config) {
+  const event = new Event('milo:deferred');


really like this idea!

Another great idea from @auniverseaway

aem-code-sync · 2023-01-13T20:39:36Z

Page	Score	PSI Audit	Google
/drafts/rclayton/carousel-demo?martech=off

ryanmparrish

Looks and works great, I've got nothing. Nice job!

aem-code-sync · 2023-01-19T20:20:40Z

Page	Score	PSI Audit	Google
/drafts/rclayton/carousel-demo?martech=off

* MWPW-122131 - Embed Blocks (#348) * add twitter auto_block * add tiktok * add tiktok test * add vimeo * add instagram * add slideshare * minor fix * fix vimeo player link * fix vimeo url check * PR feedback Co-authored-by: Chris Millar <cmillar@adobe.com> Co-authored-by: Ryan Clayton <rgclayton@gmail.com> * MWPW-120782 - Fix Chart Color Order (#393) * MWPW-109331 - Carousel (#375) * Carousel v1 * PR feedback, clean up * PR feedback. Clean up. Co-authored-by: Ryan Clayton <rclayton@adobe.com> Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com> * MWPW-124631 check if adobe logo block exists in document (#406) check if adobe logo block exists in doc * Migrate Walls.io (#378) * add walls.io block * add test * fix css * fix url vulnerability * pr feedback * fix vulnerability * pr feedback * MWPW-124632 [Milo] No character count in the review component text box (#407) * MWPW-124584 (#411) Co-authored-by: Blaine Gunn <gunn@adobe.com> * MWPW-124845 - Start Project button missing for only some projects (#409) MWPW-124845 - Start Project button missing for only English Copy Projects - update start button show condition to handle the case where there are no translation projects Resolves: MWPW-124845 * MWPW-124277 - Link Localization not working for some links in Gnav and Footer (#396) - call decorateLinks for gnav in gnav init instead of individual blocks - removed decorateSVG and localizeLink instances as it is called in decorateLinks - added tests to verify link localization in gnav menu, breadcrumb, cta, brand link - call decorateLinks for footer links in init method - added try catch in utils for empty href or malformed href - fix https://business.adobe.com/au getting converted to https://business.adobe.com/au/au - add test for this fix - Support www.adobe.com link localization and add test for the same Resolves: MWPW-124277 * MWPW-124277 - Regression Fix Link Localization (#416) MWPW-124277 - Regression Fix Link Localization for - Retaining the per menu localize link to avoid regression issue noticed with few links not being localized. Resolves: MWPW-124277 * MWPW-123893 - Review Schema (#415) * Add product schema to review block * MWPW-123745 SVG alt text (#401) * MWPW-123745 SVG alt text Added feature to svg images to allow for authors to add alt text. * PR feedback Co-authored-by: Ryan Clayton <rclayton@adobe.com> Co-authored-by: Chris Millar <cmillar@adobe.com> * MWPW-120020 - Event-driven Modals (#403) * Ability to open a modal with a custom event * Ability to customize the look and behavior of a modal * Turned GeoRouting on for Milo * MWPW-1224169 Updated How-to block (#404) * MWPW-1224169 Updated How-to block * Fix tests; Update ld json * Update inline image size * Die gracefully with old format * full backwards compatibility Co-authored-by: ChrisChrisChris <chrischrischris@users.noreply.github.com> * small windows fix (#418) * MWPW-123760 (#383) Co-authored-by: Chris Millar <cmillar@adobe.com> Co-authored-by: Ryan Clayton <rgclayton@gmail.com> Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com> * MWPW-124906 - Embed links inside text block (#420) Co-authored-by: Chris Millar <cmillar@adobe.com> * MWPW-124846 - Loc Support fragments at any level in hierarchy (#419) Co-authored-by: bhagwath <bhagwath@adobe.com> * MWPW-125032 Brand Block: using an SVG link for logo breaks gnav rendering (#422) * MWPW-120802 - Diagonal Labels for Chart (#412) * MWPW-120802 - Diagonal Labels for Chart * pr * Add asset support to library * Add Franklin customer library support * Remove 3rd party libraries if not used * Add utils test coverage (#427) Co-authored-by: Ryan Clayton <rclayton@adobe.com> * A simple `reading-time` block to estimate the reading time of the article (#356) * A simple block to estimate the reading time of the article * reading-time: typo in the css * reading-time: small change to the css * reading-time PR review changes * added placeholders for localization of the '5 min read' text * [reading-time]: add a placeholder key for singular reading time value * [reading-time]: less strict scoping for css * [reading-time]: less strict scoping for css * [reading-time]: text-align center only if it's in a center section * reading-time tests added * reading-time tests added * Fixing bug for faas import from link clicked. (#424) * Fixing bug for faas import from link clicked. * adding ||= sytex. * same as before. * MWPW-122611 - [GH Action] Old browser function detection (#423) * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions * MWPW-122611 - [GH Action] Old browser function detection - add a Github action to check for unsupported functions --------- Co-authored-by: Aaron Mauchley <mauchley@adobe.com> * removing display none from accordion (#433) --------- Co-authored-by: Saugat Malla <saugat013@gmail.com> Co-authored-by: Chris Millar <cmillar@adobe.com> Co-authored-by: Ryan Clayton <rgclayton@gmail.com> Co-authored-by: Megan Thomas <methomas@adobe.com> Co-authored-by: Ryan Clayton <rclayton@adobe.com> Co-authored-by: Chris Peyer <chrischrischris@users.noreply.github.com> Co-authored-by: Mirko Sagolj <37147400+msagolj@users.noreply.github.com> Co-authored-by: Blaine Gunn <Blainegunn@gmail.com> Co-authored-by: Blaine Gunn <gunn@adobe.com> Co-authored-by: Janaki R Bhagwath <janaki.r.bhagwath@gmail.com> Co-authored-by: bhagwath <bhagwath@adobe.com> Co-authored-by: Solène <62023521+So1ene@users.noreply.github.com> Co-authored-by: Sean Choi <seanchoi@adobe.com> Co-authored-by: jckautzmann <jkautzma@adobe.com> Co-authored-by: Aaron Mauchley <mauchley@adobe.com> Co-authored-by: Brad Johnson <fullcolorcoder@gmail.com>

Carousel v1

74140e5

chrischrischris reviewed Jan 9, 2023

View reviewed changes

PR feedback, clean up

bdf827a

rgclayton requested a review from ryanmparrish January 10, 2023 20:48

Merge branch 'main' into rclayton/carousel-v1

fb4c5bc

vhargrave approved these changes Jan 12, 2023

View reviewed changes

PR feedback. Clean up.

cf7f308

ryanmparrish approved these changes Jan 17, 2023

View reviewed changes

Merge branch 'main' into rclayton/carousel-v1

db0b925

rgclayton added the high-impact Any PR that may affect consumers label Jan 19, 2023

auniverseaway changed the title ~~Carousel v1~~ MWPW-109331 - Carousel Jan 19, 2023

auniverseaway added the verified PR has been E2E tested by a reviewer label Jan 20, 2023

rgclayton merged commit ec77160 into main Jan 20, 2023

rgclayton deleted the rclayton/carousel-v1 branch January 20, 2023 00:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MWPW-109331 - Carousel #375

MWPW-109331 - Carousel #375

rgclayton commented Jan 7, 2023 •

edited by auniverseaway

Loading

aem-code-sync bot commented Jan 7, 2023

aem-code-sync bot commented Jan 7, 2023

codecov bot commented Jan 7, 2023 •

edited

Loading

chrischrischris Jan 9, 2023

chrischrischris Jan 9, 2023

rgclayton Jan 9, 2023

chrischrischris Jan 11, 2023

rgclayton Jan 13, 2023

aem-code-sync bot commented Jan 10, 2023

aem-code-sync bot commented Jan 11, 2023

vhargrave left a comment

vhargrave Jan 12, 2023

rgclayton Jan 13, 2023

vhargrave Jan 12, 2023

rgclayton Jan 13, 2023

aem-code-sync bot commented Jan 13, 2023

ryanmparrish left a comment

aem-code-sync bot commented Jan 19, 2023

	btnsArray.push(previousBtn, nextBtn);
	return btnsArray;
	return [previousBtn, nextBtn];


		el.append(...nextPreviousBtns, controlsContainer);

		parentArea.addEventListener('milo:deferred', () => {

MWPW-109331 - Carousel #375

MWPW-109331 - Carousel #375

Conversation

rgclayton commented Jan 7, 2023 • edited by auniverseaway Loading

aem-code-sync bot commented Jan 7, 2023

aem-code-sync bot commented Jan 7, 2023

codecov bot commented Jan 7, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aem-code-sync bot commented Jan 10, 2023

aem-code-sync bot commented Jan 11, 2023

vhargrave left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aem-code-sync bot commented Jan 13, 2023

ryanmparrish left a comment

Choose a reason for hiding this comment

aem-code-sync bot commented Jan 19, 2023

rgclayton commented Jan 7, 2023 •

edited by auniverseaway

Loading

codecov bot commented Jan 7, 2023 •

edited

Loading