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

YALB-1697: Bug: Margin for spotlights #332

Merged
merged 18 commits into from
Feb 26, 2024
Merged

YALB-1697: Bug: Margin for spotlights #332

merged 18 commits into from
Feb 26, 2024

Conversation

joetower
Copy link
Contributor

@joetower joetower commented Feb 2, 2024

YALB-1697: Bug: Margin for spotlights

Description of work

  • Adds JS to scan the page for consecutive instances of text-with-image or content-spotlight-portrait
  • If there are two consecutive instances, the first will have margin applied to the top and the second will have margin applied to the bottom.
  • If there are several consecutive instances, the first will have top margin and the last will have bottom margin.
  • If a spotlight uses a default / no color theme option, it will have top and bottom margin, even if it is the only spotlight instance
  • Spotlights, by default, do not have margins. Instead, we introduce margins if specific attributes are present.
  • Margins are introduced if:
    • the spotlight does not use a theme color (is set to default)
    • the spotlight is the only spotlight on the page
    • there are consecutive spotlights on the page, the first has top margin, the last has bottom margin

Testing Link(s)

Functional Review Steps

yalb-spotlight-margins-drupal.mp4
  • Note that if either Content Spotlights, landscape or portrait, are using a theme and they are the last component before the footer, there is no bottom margin.
  • Note that if you add a Content Spotlight set to Theme: Default - No Theme, there is space between the component and the footer.
  • With one spotlight : https://pr-563-yalesites-platform.pantheonsite.io/one-spotlight-only
  • Note: if a spotlight is the one and only spotlight on the page, then it should have top and bottom margin (data-spotlights-position="first-and-last")
yalb-spotlight-margins-drupal-single.mp4

@joetower joetower self-assigned this Feb 2, 2024
@joetower joetower marked this pull request as draft February 2, 2024 18:51
Copy link

netlify bot commented Feb 2, 2024

Deploy Preview for dev-component-library-twig ready!

Name Link
🔨 Latest commit 34f1b76
🔍 Latest deploy log https://app.netlify.com/sites/dev-component-library-twig/deploys/65dc07f7405852000801e700
😎 Deploy Preview https://deploy-preview-332--dev-component-library-twig.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@joetower joetower changed the title feat(YALB-1697): wip - margins on spotlight components YALB-1697: Bug: Margin for spotlights Feb 12, 2024
Copy link
Contributor

@dblanken-yale dblanken-yale left a comment

Choose a reason for hiding this comment

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

Works great! Nice work! I was going to suggest maybe combining the if statements at the end so that it doesn't overwrite itself in the event it is first and last, but then again that means there's only one spotlight and that's a trivial performance gain. LOL

I really like the idea of creating the data attributes for this! So easy to understand in the markup.

@sskalisky-4k
Copy link

This looks great, @joetower!

Copy link

@miketullo95 miketullo95 left a comment

Choose a reason for hiding this comment

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

this is great!

Copy link
Contributor

@codechefmarc codechefmarc left a comment

Choose a reason for hiding this comment

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

Whoops, reviewed on Atomic as well, I think that's correct. Anyways, approved! Nice work!

@nJim nJim merged commit d1d0676 into develop Feb 26, 2024
5 checks passed
@nJim nJim deleted the YALB-1697-margins branch February 26, 2024 03:45
@vinmassaro vinmassaro mentioned this pull request Feb 26, 2024
Copy link

🎉 This PR is included in version 1.39.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

6 participants