Skip to content

GAUD-10312 - No arrows when divider collapsed #7341

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.

Sign up for GitHub

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

Merged
svanherk merged 2 commits into from
Aug 10, 2026
Merged

GAUD-10312 - No arrows when divider collapsed #7341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f18b36
GAUD-10312 - No arrows when divider collapsed
svanherk Aug 10, 2026
e582007
Updating vdiff goldens (#7342)
github-actions[bot] Aug 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions components/page/page-divider-internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,13 @@ class PageDivider extends FocusMixin(PropertyRequiredMixin(LitElement)) {
`;
}

#clickedArrow;
#clickedHandle = false;

#getArrowVisibility() {
if (this.panelType !== 'panel') return { showStartArrow: false, showEndArrow: false };
const canShrink = !this.collapsed && this.currentSize > this.minSize;
const canGrow = this.collapsed || this.currentSize < this.maxSize;
if (this.panelType !== 'panel' || this.collapsed) return { showStartArrow: false, showEndArrow: false };
const canShrink = this.currentSize > this.minSize;
const canGrow = this.currentSize < this.maxSize;
const showStartArrow = this.panelPosition === 'start' ? canShrink : canGrow;
const showEndArrow = this.panelPosition === 'start' ? canGrow : canShrink;
return { showStartArrow, showEndArrow };
Expand All @@ -265,10 +266,18 @@ class PageDivider extends FocusMixin(PropertyRequiredMixin(LitElement)) {
}

#handleClick(e) {
// Do not toggle until click event is received, to avoid clicking on elements under the handle in drawer mode
// Do not toggle/resize until click event is received,
// to avoid clicking on elements under the arrows in overlay mode or under the handle in drawer mode
e.stopPropagation();
if (this.collapsed || this.#clickedHandle) {
this.#sendToggleEvent();
} else if (this.#clickedArrow) {
const endArrowClicked = this.#clickedArrow.dataset.position === 'end';
const shouldGrow = endArrowClicked === (this.panelPosition === 'start');

const step = (shouldGrow ? 1 : -1) * KEYBOARD_STEP;
const requestedSize = this.currentSize + step;
this.#sendResizeEvent(requestedSize);
}
}

Expand Down Expand Up @@ -313,17 +322,10 @@ class PageDivider extends FocusMixin(PropertyRequiredMixin(LitElement)) {

const path = e.composedPath();
this.#clickedHandle = path.some(el => el.classList?.contains('divider-handle'));
this.#clickedArrow = path.find(el => el.classList?.contains('divider-arrow'));
if (this.#clickedArrow) return; // Arrows don't support dragging

const clickedArrow = path.find(el => el.classList?.contains('divider-arrow'));
if (clickedArrow) {

@svanherk svanherk Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copied from other PR for posterity:

This code moving into the click handler above fixes an eventual bug that would have come up with overlay. It's the same issue the drawer would have had - if we move the divider before the click happens, it'll click what's behind it. I thought this was only an issue with the drawer handle, but it also happens to the arrows in overlay mode, where clicking the overlay closes the drawer.

Even if we don't want that to happen (I need to ask design), it's just better and will likely save us from some other bug in the future.

const endArrowClicked = clickedArrow.dataset.position === 'end';
const shouldGrow = endArrowClicked === (this.panelPosition === 'start');

const step = (shouldGrow ? 1 : -1) * KEYBOARD_STEP;
const requestedSize = this.currentSize + step;
this.#sendResizeEvent(requestedSize);
return;
}
// TO DO: Dragging
}

#sendResizeEvent(requestedSize) {
Expand Down
Binary file modified ...e/test/golden/page-divider-internal/chromium/click-handle-collapse-side-nav.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...n/page-divider-internal/chromium/click-handle-collapse-supporting-immersive.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...golden/page-divider-internal/chromium/collapse-stay-scrolled-immersive-main.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...page/test/golden/page-divider-internal/chromium/collapse-stay-scrolled-main.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file removed ...e/test/golden/page-divider-internal/chromium/hover-collapsed-arrow-side-nav.png
View file
Open in desktop
Binary file not shown.
Binary file removed ...n/page-divider-internal/chromium/hover-collapsed-arrow-supporting-immersive.png
View file
Open in desktop
Binary file not shown.
19 changes: 5 additions & 14 deletions components/page/test/page-divider-internal.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,24 +202,15 @@ describe('d2l-page-divider-internal', () => {
});
});

it('shrink arrow does not appear when collapsed', async() => {
it('both arrows are hidden when collapsed', async() => {
const elem = await fixture(
createDivider({ collapsed: true, currentSize, panelPosition: test.panelPosition }),
{ rtl: test.rtl }
);
const arrowElem = getDividerArrow(elem, test.shrinkArrow);
expect(arrowElem.hidden).to.be.true;
});

it('grow arrow appears and requests a resize to min size when collapsed', async() => {
const elem = await fixture(
createDivider({ collapsed: true, currentSize: 0, panelPosition: test.panelPosition }),
{ rtl: test.rtl }
);
await focusElem(elem);
clickElem(getDividerArrow(elem, test.growArrow));
const e = await oneEvent(elem, 'd2l-page-divider-resize');
expect(e.detail.requestedSize).to.equal(minSize);
const shrinkArrowElem = getDividerArrow(elem, test.shrinkArrow);
expect(shrinkArrowElem.hidden).to.be.true;
const growArrowElem = getDividerArrow(elem, test.growArrow);
expect(growArrowElem.hidden).to.be.true;
});
});
});
Expand Down
14 changes: 0 additions & 14 deletions components/page/test/page-divider-internal.vdiff.js
View file
Open in desktop

@svanherk svanherk Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These tests no longer provide value, as there's nothing to hover.

Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,6 @@ describe('page-divider-internal', () => {
await expect(elem).to.be.golden({ margin: 0 });
});
});

[
{ name: 'arrow-side-nav', arrow: 'end', divider: 'side-nav', fixture: pageDividerFixtures.sideNavBothHeadersFooterStorageKey },
{ name: 'arrow-supporting-immersive', arrow: 'start', divider: 'supporting', fixture: pageDividerFixtures.supportingImmersiveBothHeadersStorageKey }
].forEach(test => {
it(test.name, async() => {
setStoredPanelState({ [test.divider]: { size: 400, collapsed: true } });
const elem = await fixture(test.fixture, { pagePadding: false, viewport: { width: 1000, height: 400 } });
const divider = getDivider(elem, test.divider);
await focusElem(divider);
await hoverElem(getDividerArrow(divider, test.arrow));
await expect(elem).to.be.golden({ margin: 0 });
});
});
});

describe('focus', () => {
Expand Down