fix: ac-scroll fills remaining height in mobile overlay#43
Conversation
Replaced max-height: 40vh cap with flex: 1; min-height: 0 so the autocomplete list expands to fill whatever height remains after the back bar, search input, chip bar, and facet bar. Panel becomes a flex column while keeping overflow: visible so facet dropdowns aren't clipped.
There was a problem hiding this comment.
Pull request overview
Adjusts the ol-search-bar mobile full-screen overlay layout so the autocomplete results list (.ac-scroll) expands to fill the remaining viewport height, eliminating the blank space previously caused by a fixed vh max-height.
Changes:
- Make
:host(.mobile-exp) .panela column flex container to create a defined vertical layout. - Update
:host(.mobile-exp) .ac-scrolltoflex: 1(withmin-height: 0) so it consumes remaining space and scrolls naturally. - Update the mobile overlay CSS contract test to assert flex-based growth instead of a
vhmax-height.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/components/ol-search-bar.js | Converts the mobile overlay panel to a column flex layout and makes .ac-scroll fill remaining height with scrolling. |
| frontend/src/components/ol-search-bar.mobile-overlay.test.js | Updates the static CSS contract assertion to match the new flex-based overlay behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| :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; } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Two bugs reported after the initial flex fill: 1. Body behind position:fixed overlay was still scrollable — visually a full-page scrollbar appeared. Fix: lock document.body.style.overflow when _mobileExpanded, restore it on close or disconnect. 2. Shrinking the browser window clipped content at the bottom rather than scrolling — .search-outer and .panel lacked min-height:0 so the flex chain couldn't constrain their heights. Fix: add min-height:0 to both; change .panel overflow to hidden (safe in full-screen mode since facet dropdowns stay within the panel's viewport-sized bounds).
…ight tests Address Copilot review comments on PR #43: - Scope @media(max-width:600px) .ac-scroll to :host(:not(.mobile-exp)) so overlay and non-overlay modes are explicitly separate rather than relying on selector specificity to determine which max-height wins. - Add contract tests asserting .panel is display:flex/flex-direction:column and .ac-scroll has min-height:0 in mobile-exp, protecting the full flex layout intent against future regressions.
Summary
.ac-scrollwas capped atmax-height: 40vh, leaving blank white space below the autocomplete list.panelaflex-direction: columncontainer so children stack in a defined-height layout.ac-scrollflex: 1; min-height: 0so it grows to fill whatever height remains after the back-bar, search input, chip row, and facet bar take their natural heightsoverflow: visibleon.panel— facet dropdowns are positioned children that must not be clippedTest plan
npx vitest run)