Skip to content

MWPW-192327 Fix color popover appearing under nav#372

Merged
meganthecoder merged 1 commit intostagefrom
methomas/popover-nav
Apr 17, 2026
Merged

MWPW-192327 Fix color popover appearing under nav#372
meganthecoder merged 1 commit intostagefrom
methomas/popover-nav

Conversation

@meganthecoder
Copy link
Copy Markdown
Contributor

Summary

  • Fixes bug where Edit Color popover would underlap the nav if there wasn't enough space in the viewport below. Now it opens below the button and avoids the nav.
  • Removes hexcode button background when popover closes.

Jira Ticket

Resolves: MWPW-192327


Test URLs

Env URL
Before https://stage--express-color--adobecom.aem.page/create/color-wheel
After https://methomas-popover-nav--express-color--adobecom.aem.page/create/color-wheel

Verification Steps

  • Change your window height to 724px.
  • Add colors to get a 2-row layout.
  • Click the hexcode button to open the popover.
  • Popover should open below the button.

Potential Regressions


Additional Notes

(If applicable) Add context, related PRs, or known issues here.

@aem-code-sync
Copy link
Copy Markdown

aem-code-sync Bot commented Apr 16, 2026

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

@github-actions github-actions Bot added the Ready for Review Ready for peer review. label Apr 16, 2026
@aem-code-sync
Copy link
Copy Markdown

aem-code-sync Bot commented Apr 16, 2026

Page Scores Audits
📱 /create/color-accessibility PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS
🖥️ /create/color-accessibility PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS
📱 /create/color-wheel PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS
🖥️ /create/color-wheel PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS

Copy link
Copy Markdown
Contributor

@fullcolorcoder fullcolorcoder left a comment

Choose a reason for hiding this comment

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

Image Image

In a few scenarios it isn't making sense I think, and still cuts off.

Also, on scroll things become inaccessible as well.

Nit suggestion: Make it more dynamic even on scroll

At minimum we should refine to not be cut off by default.

function setupPopoverAutoPositioning(popover, anchorElement, container) {
  let rafId = 0;

  const scheduleUpdate = () => {
    cancelAnimationFrame(rafId);
    rafId = requestAnimationFrame(() => {
      if (!popover.isConnected) return;
      positionPopover(popover, anchorElement.getBoundingClientRect(), container);
    });
  };

  const headerElements = [
    document.querySelector('header.global-navigation'),
    document.querySelector('.feds-localnav'),
  ].filter(Boolean);

  const resizeObserver = new ResizeObserver(scheduleUpdate);
  resizeObserver.observe(popover);
  resizeObserver.observe(anchorElement);
  headerElements.forEach((el) => resizeObserver.observe(el));

  const intersectionObserver = new IntersectionObserver(scheduleUpdate, {
    root: null,
    threshold: [0, 0.25, 0.5, 0.75, 1],
  });
  intersectionObserver.observe(anchorElement);

  window.addEventListener('scroll', scheduleUpdate, true);
  window.addEventListener('resize', scheduleUpdate);

  scheduleUpdate();

  return () => {
    cancelAnimationFrame(rafId);
    resizeObserver.disconnect();
    intersectionObserver.disconnect();
    window.removeEventListener('scroll', scheduleUpdate, true);
    window.removeEventListener('resize', scheduleUpdate);
  };
}

function positionPopover(popover, anchorRect, container) {
  const gap = 4;
  const popRect = popover.getBoundingClientRect();
  const containerRect = container.getBoundingClientRect();
  const headerBottom = getStickyHeaderBottom();

  const belowTop = anchorRect.bottom + gap;
  const aboveTop = anchorRect.top - popRect.height - gap;

  const fitsBelow = belowTop + popRect.height <= window.innerHeight;
  const fitsAbove = aboveTop >= headerBottom;

  let viewportTop = belowTop;
  if (!fitsBelow && fitsAbove) {
    viewportTop = aboveTop;
  }

  // keep existing left/clamping logic here...
}

@meganthecoder
Copy link
Copy Markdown
Contributor Author

@fullcolorcoder Can you explain a little more the behavior you're looking for? The screenshots you included look like the expected behavior. When there isn't enough space either above or below the button, we have it show up below because a user can scroll down to access it.

@fullcolorcoder
Copy link
Copy Markdown
Contributor

@meganthecoder My suggestion is to ensure that we don't force a user to navigate the page to show the menu when that can be avoided, similarly to the fix here avoiding forcing the user to scroll down as well. This is improved from what it was but is revealing to what could be refined further.

@meganthecoder
Copy link
Copy Markdown
Contributor Author

@fullcolorcoder Got it. I'll merge as-is since it covers the ask, and I can have a discussion with Julia about future enhancements to the behavior.

@meganthecoder meganthecoder merged commit da6e50c into stage Apr 17, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for Review Ready for peer review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants