Skip to content

fix: ac-scroll fills remaining height in mobile overlay #43

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
mekarpeles merged 3 commits into from
Apr 27, 2026
Merged

fix: ac-scroll fills remaining height in mobile overlay #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1f12e5
fix: ac-scroll fills remaining viewport height in mobile overlay
mekarpeles Apr 27, 2026
cbfaa6f
fix: prevent body scroll and fix flex height chain in mobile overlay
mekarpeles Apr 27, 2026
9331633
fix: scope 600px ac-scroll rule to :not(.mobile-exp); add flex/min-he…
mekarpeles Apr 27, 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
15 changes: 10 additions & 5 deletions frontend/src/components/ol-search-bar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export class OlSearchBar extends LitElement {
super.disconnectedCallback();
document.removeEventListener('click', this._onDoc, true);
window.removeEventListener('resize', this._onWinResize);
if (this._mobileExpanded) document.body.style.overflow = '';
this._acAbort?.abort();
this._authorAbort?.abort();
this._subjectAbort?.abort();
Expand All @@ -147,6 +148,10 @@ export class OlSearchBar extends LitElement {
}
// Sync full-screen overlay class on the host element.
this.classList.toggle('mobile-exp', this._mobileExpanded);
// Prevent body scroll while overlay is active so the page behind doesn't scroll.
if (changed.has('_mobileExpanded')) {
document.body.style.overflow = this._mobileExpanded ? 'hidden' : '';
}
// Anchor the panel to the trigger and focus the panel-input when it opens.
if (changed.has('_open') && this._open && this.showFacets) {
if (!this._mobileExpanded) this._positionPanel();
Expand Down Expand Up @@ -665,18 +670,18 @@ export class OlSearchBar extends LitElement {
background: white; overflow: hidden;
}
:host(.mobile-exp) .search-outer {
flex: 1; display: flex; flex-direction: column;
flex: 1; min-height: 0; display: flex; flex-direction: column;
}
:host(.mobile-exp) .input-row { display: none; }
:host(.mobile-exp) .panel {
position: static; flex: 1;
position: static; flex: 1; min-height: 0;
width: 100%; box-sizing: border-box;
overflow: visible; max-height: none;
display: flex; flex-direction: column; overflow: hidden; max-height: none;
border: none; box-shadow: none; border-radius: 0;
border-top: none;
}
:host(.mobile-exp) .panel-chips { max-height: none; }
:host(.mobile-exp) .ac-scroll { max-height: 40vh; }
:host(.mobile-exp) .ac-scroll { flex: 1; min-height: 0; max-height: none; overflow-y: auto; }
:host(.mobile-exp) .pf-bar { overflow: visible; flex-wrap: wrap; }
Comment on lines +684 to 685

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

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

At <=600px there is still a later @media (max-width: 600px) { .ac-scroll { max-height: 40vh; } } rule, which now conflicts with this :host(.mobile-exp) .ac-scroll { max-height: none; ... } change. It works today due to selector specificity, but it’s fragile/confusing—consider scoping the 600px rule to non-overlay (e.g. :host(:not(.mobile-exp)) .ac-scroll) or otherwise removing/rewriting it so overlay/non-overlay behavior is explicit.

Copilot uses AI. Check for mistakes.

@mekarpeles mekarpeles Apr 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 9331633: scoped the rule to :host(:not(.mobile-exp)) .ac-scroll so overlay and non-overlay modes are explicitly separate — no more relying on specificity ordering to determine which max-height wins.


/* Back button shown at top of the expanded panel */
Expand All @@ -697,7 +702,7 @@ export class OlSearchBar extends LitElement {
.pf-bar::-webkit-scrollbar { display: none; }
.pf-btn { font-size: 10px; padding: 11px 4px; }
.submit { padding: 6px 10px; }
.ac-scroll { max-height: 40vh; }
:host(:not(.mobile-exp)) .ac-scroll { max-height: 40vh; }
.panel-chips { max-height: 72px; overflow-y: auto; }
}
`;
Expand Down
42 changes: 38 additions & 4 deletions frontend/src/components/ol-search-bar.mobile-overlay.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,39 @@ describe('ol-search-bar mobile overlay CSS contract', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*max-height\s*:\s*none/);
});

it(':host(.mobile-exp) .ac-scroll has a vh-based max-height for results scrolling', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.ac-scroll[^}]*max-height\s*:\s*\d+vh/);
it(':host(.mobile-exp) .ac-scroll grows to fill remaining panel height via flex: 1', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.ac-scroll[^}]*flex\s*:\s*1/);
Comment thread
mekarpeles marked this conversation as resolved.
});

it(':host(.mobile-exp) .ac-scroll has min-height:0 so it can shrink below content size', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.ac-scroll[^}]*min-height\s*:\s*0/);
});

it(':host(.mobile-exp) .panel is a flex column so children stack and ac-scroll can flex-grow', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*display\s*:\s*flex/);
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*flex-direction\s*:\s*column/);
});

it('@media 600px .ac-scroll rule is scoped to :host(:not(.mobile-exp)) — no specificity conflict', () => {
const mediaBlock = src.slice(src.indexOf('@media (max-width: 600px)'), src.indexOf('@media (max-width: 600px)') + 400);
expect(mediaBlock).toMatch(/:host\(:not\(\.mobile-exp\)\)\s+\.ac-scroll/);
expect(mediaBlock).not.toMatch(/[^)]\s+\.ac-scroll\s*\{[^}]*max-height/); // plain .ac-scroll should not appear
});

it(':host(.mobile-exp) .panel sets width:100% to override the desktop CSS var', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*width\s*:\s*100%/);
});

it(':host(.mobile-exp) .panel uses overflow:visible so facet dropdowns are not clipped', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*overflow\s*:\s*visible/);
it(':host(.mobile-exp) .panel uses overflow:hidden to bound the flex scroll region', () => {
// In full-screen mode the panel IS the viewport, so facet dropdowns remain
// within its bounds and are not clipped; overflow:hidden is needed so that
// flex children (ac-scroll) are properly height-constrained.
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*overflow\s*:\s*hidden/);
});

it(':host(.mobile-exp) .panel and .search-outer have min-height:0 so flex chain can shrink', () => {
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.panel[^}]*min-height\s*:\s*0/);
expect(src).toMatch(/:host\(\.mobile-exp\)\s+\.search-outer[^}]*min-height\s*:\s*0/);
});
});

Expand Down Expand Up @@ -82,4 +105,15 @@ describe('ol-search-bar mobile overlay JS contract', () => {
it('toggles mobile-exp class on :host via updated()', () => {
expect(src).toMatch(/classList\.toggle\s*\(\s*['"]mobile-exp['"]\s*,\s*this\._mobileExpanded\s*\)/);
});

it('locks body scroll when mobile overlay opens and restores it when it closes', () => {
const updatedFn = src.slice(src.indexOf('updated(changed)'), src.indexOf('updated(changed)') + 1200);
expect(updatedFn).toMatch(/document\.body\.style\.overflow/);
expect(updatedFn).toMatch(/_mobileExpanded.*hidden|hidden.*_mobileExpanded/s);
});

it('restores body scroll in disconnectedCallback in case component is removed while expanded', () => {
const dcFn = src.slice(src.indexOf('disconnectedCallback()'), src.indexOf('disconnectedCallback()') + 400);
expect(dcFn).toMatch(/document\.body\.style\.overflow/);
});
});
2 changes: 1 addition & 1 deletion frontend/src/components/ol-search-bar.panel-overlay.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ describe('ol-search-bar panel overlay — panel-input contract', () => {
});

it('updated() focuses the panel-input when the panel opens', () => {
const updFn = src.slice(src.indexOf('updated(changed)'), src.indexOf('updated(changed)') + 1000);
const updFn = src.slice(src.indexOf('updated(changed)'), src.indexOf('updated(changed)') + 1200);
expect(updFn).toMatch(/panel-input/);
});
});
Expand Down
Loading