Skip to content

Add Full Screen carousel variant#559

Closed
RitwikSrivastava wants to merge 5 commits intomainfrom
carouselwide
Closed

Add Full Screen carousel variant#559
RitwikSrivastava wants to merge 5 commits intomainfrom
carouselwide

Conversation

@RitwikSrivastava
Copy link
Copy Markdown
Collaborator

@RitwikSrivastava RitwikSrivastava commented Mar 17, 2025

Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):

Fix #552

Test URLs:

@aem-code-sync
Copy link
Copy Markdown

aem-code-sync bot commented Mar 17, 2025

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

@aem-code-sync
Copy link
Copy Markdown

aem-code-sync bot commented Mar 17, 2025

Page Scores Audits Google
📱 /latino-es/service/centroamerica PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /latino-es/service/centroamerica PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@RitwikSrivastava RitwikSrivastava changed the title Add new carousel variant Full Scree carousel variant Mar 17, 2025
@RitwikSrivastava RitwikSrivastava changed the title Full Scree carousel variant Add Full Screen carousel variant Mar 17, 2025
Copy link
Copy Markdown
Collaborator

@helms-charity helms-charity left a comment

Choose a reason for hiding this comment

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

There are a couple of changes inline you can try so the image isn't cut off, but also please display:none this container in widths less than 768px wide, like the live site does.


.carousel.wide .carousel-slide .carousel-slide-image picture > img {
width: 100%;
height: 100%;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remove height since you have object-fit: cover.

/* stylelint-disable-next-line no-descending-specificity */
.carousel.wide .carousel-slide {
min-width: 100%; /* Full width slides */
height: 400px; /* Adjust height as needed */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove height: 400px;
instead, add
aspect-ratio: 15 / 4;

Copy link
Copy Markdown
Collaborator

@helms-charity helms-charity 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 also make sure auto-scroll works for this page /latino-es/service/centroamerica like the original?

@RitwikSrivastava
Copy link
Copy Markdown
Collaborator Author

Can you also make sure auto-scroll works for this page /latino-es/service/centroamerica like the original?

Autoscroll also added .

@RitwikSrivastava
Copy link
Copy Markdown
Collaborator Author

Closing this. will raise a new PR in da-pilot repo as this needs some more work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Import] Full Screen carousel

2 participants