fix: stop the header overlapping itself when it runs out of room (#4906)

On a tablet-width screen the header had no way to give up space. The
navigation, the logo and the session controls each held their full width,
so once an extension added a few links the row simply overflowed and the
controls painted over each other — a username cut mid-word on an iPad in
portrait, or nav links sitting on top of the search icon.

Three things were behind it:

- `.container` was pinned to the width that opened each breakpoint, so at
  820px the layout was still 768px wide and 52px of screen went unused.
  The bands above 1100px were already fixed this way; this extends the same
  clamp down to tablet and desktop.
- `.Header-primary` declared no flex behaviour at all, so it could neither
  shrink nor grow, and `.Header-secondary` used an auto margin — which takes
  free space before flex distributes any, pooling all the slack into one gap
  mid-row.
- Nothing ever moved out of the way. Navigation now collapses what does not
  fit into a menu at the end of the row, via a new `OverflowingList`.

The header search made this worse than it needed to be. Since search moved
into a modal it has been a `readonly` text field that cannot be typed into —
reserving a text field's width for a control that only ever behaved as a
button, and announcing itself to assistive technology as a textbox that
refuses input. It is now a button, with the same icon-and-label markup as
the notification and message controls, so it collapses to its icon in the
header and keeps its label in the drawer. The locale selector gains an icon
for the same reason.

The fitting arithmetic lives in `countItemsThatFit` rather than inside the
component, so the awkward cases can be tested directly: boundaries where an
item exactly fits, keeping room for the toggle that is about to appear, and
the monotonicity that stops the row oscillating between two states.

`OverflowingList` measures real layout, which jsdom cannot provide, so the
component tests cover the markup either side of the decision and the sums
are covered on their own.

Author: imorland

framework/core/js/src/common/components/AbstractGlobalSearch.tsx

@@ -3,7 +3,7 @@ import Component, { ComponentAttrs } from '../Component';
 import SearchState from '../states/SearchState';
 import extractText from '../utils/extractText';
 import ItemList from '../utils/ItemList';
-import Input from './Input';
+import Icon from './Icon';
 import type Mithril from 'mithril';
 
 export interface SearchAttrs extends ComponentAttrs {
@@ -90,50 +90,51 @@ export default abstract class AbstractGlobalSearch<T extends SearchAttrs = Searc
     // Hide the search view if no sources were loaded
     if (this.sourceItems().isEmpty()) return <div></div>;
 
+    const value = this.searchState.getValue();
+
+    // Search happens in a modal, so this control does not accept text: it is a
+    // button that opens the modal, and reads as one to assistive technology
+    // rather than as a textbox that refuses input. Where its label is shown —
+    // the drawer — it displays the current query in place of the prompt, so a
+    // search that has been run stays visible after the modal closes.
     const openSearchModal = () => {
-      this.$('input').blur() &&
-        app.modal.show(() => import('../../common/components/SearchModal'), { searchState: this.searchState, sources: this.sourceItems().toArray() });
+      app.modal.show(() => import('../../common/components/SearchModal'), { searchState: this.searchState, sources: this.sourceItems().toArray() });
     };
 
     return (
-      <div
-        role="search"
-        className="Search"
-        aria-label={this.attrs.a11yRoleLabel}
-        onclick={() => {
-          // On phones the search input lives inside the slide-out drawer. Close the drawer as soon as
-          // search is activated, before the modal is shown, so it isn't left open behind (and overlapping)
-          // the full-screen search modal as it animates in. Hiding it here rather than in `openSearchModal`
-          // lets the drawer finish sliding out during the delay below. No-op where the drawer is never open.
-          app.drawer.hide();
-          this.$('input').blur();
-          setTimeout(() => openSearchModal(), 150);
-        }}
-      >
-        <Input
-          type="search"
-          className="Search-input"
-          clearable={this.searchState.getValue()}
-          clearLabel={app.translator.trans('core.lib.search.search_clear_button_accessible_label')}
-          prefixIcon="fas fa-search"
-          aria-label={this.attrs.label}
-          readonly={true}
-          placeholder={this.attrs.label}
-          value={this.searchState.getValue()}
-          onchange={(value: string) => {
-            if (!value) this.searchState.clear();
-            else this.searchState.setValue(value);
-          }}
-          inputAttrs={{
-            // for keyboard navigation, click event would be triggered on keydown
-            onkeydown: (e: KeyboardEvent) => {
-              if (e.key === 'Enter') {
-                e.preventDefault();
-                this.$('input').blur() && openSearchModal();
-              }
-            },
+      <div role="search" className="Search" aria-label={this.attrs.a11yRoleLabel}>
+        <button
+          type="button"
+          // `Button--flat` rather than `Button--link`: this sits among the
+          // notification and message controls, which are flat buttons, and
+          // should pick up the same hover and focus treatment as them.
+          className="Search-input Button Button--flat"
+          aria-label={extractText(this.attrs.label)}
+          title={extractText(this.attrs.label)}
+          onclick={() => {
+            // On phones the search button lives inside the slide-out drawer. Close the drawer as soon as
+            // search is activated, before the modal is shown, so it isn't left open behind (and overlapping)
+            // the full-screen search modal as it animates in. Hiding it here rather than in `openSearchModal`
+            // lets the drawer finish sliding out during the delay below. No-op where the drawer is never open.
+            app.drawer.hide();
+            setTimeout(() => openSearchModal(), 150);
           }}
-        />
+        >
+          <Icon name="fas fa-search" className="Button-icon" />
+          <span className="Button-label">
+            <span className="Button-labelText">{value || this.attrs.label}</span>
+          </span>
+        </button>
+        {!!value && (
+          <button
+            type="button"
+            className="Search-clear Button Button--icon Button--link"
+            aria-label={extractText(app.translator.trans('core.lib.search.search_clear_button_accessible_label'))}
+            onclick={() => this.searchState.clear()}
+          >
+            <Icon name="fas fa-times-circle" />
+          </button>
+        )}
       </div>
     );
   }

framework/core/js/src/common/components/OverflowingList.tsx

@@ -0,0 +1,232 @@
+import Component, { ComponentAttrs } from '../Component';
+import Dropdown from './Dropdown';
+import listItems, { ModdedChildrenWithItemName } from '../helpers/listItems';
+import classList from '../utils/classList';
+import countItemsThatFit from '../utils/countItemsThatFit';
+import extractText from '../utils/extractText';
+import app from '../app';
+import type Mithril from 'mithril';
+
+export interface IOverflowingListAttrs extends ComponentAttrs {
+  /** The items to lay out, as returned by `ItemList.toArray()`. */
+  items: ModdedChildrenWithItemName[];
+  /** A class name to apply to the list element. */
+  className?: string;
+  /** The label used to describe the overflow menu to assistive readers. */
+  accessibleToggleLabel?: string;
+}
+
+/**
+ * Lays out a list of items on a single row, moving those that do not fit into
+ * a dropdown at the end of the row.
+ *
+ * The list is rendered in full and measured after each paint, so item widths
+ * come from the real laid-out DOM rather than an estimate. That keeps the
+ * result correct whatever an item happens to contain — an icon, a long label,
+ * a translated string, or something an extension added.
+ */
+export default class OverflowingList extends Component<IOverflowingListAttrs> {
+  /**
+   * How many leading items are shown in the row. `null` means "not measured
+   * yet", during which everything renders so the widths can be read.
+   */
+  protected visibleCount: number | null = null;
+
+  /**
+   * Width of each item, by index, captured while all of them were on the row.
+   * Measuring once and reusing avoids feeding the widths of an already
+   * collapsed row back into the next calculation.
+   */
+  protected itemWidths: number[] = [];
+
+  protected toggleWidth = 0;
+
+  protected observer?: ResizeObserver;
+
+  protected onWindowResize?: () => void;
+
+  view(vnode: Mithril.Vnode<IOverflowingListAttrs, this>) {
+    const items = this.attrs.items || [];
+    const count = this.visibleCount ?? items.length;
+    const visible = items.slice(0, count);
+    const overflowed = items.slice(count);
+
+    return (
+      <ul className={classList('OverflowingList', this.attrs.className)}>
+        {listItems(visible)}
+        {overflowed.length > 0 && (
+          <li className="OverflowingList-toggle" itemName="overflow">
+            <Dropdown
+              className="OverflowingList-dropdown"
+              buttonClassName="Button Button--link"
+              menuClassName="Dropdown-menu--right"
+              icon="fas fa-ellipsis-h"
+              // The ellipsis already says "there is more here"; the caret a
+              // dropdown adds by default only doubles up on that. An empty
+              // string rather than null, since the default is applied with
+              // `??=` and would overwrite a nullish value.
+              caretIcon=""
+              accessibleToggleLabel={
+                this.attrs.accessibleToggleLabel ?? extractText(app.translator.trans('core.lib.overflowing_list.toggle_accessible_label'))
+              }
+            >
+              {overflowed}
+            </Dropdown>
+          </li>
+        )}
+      </ul>
+    );
+  }
+
+  oncreate(vnode: Mithril.VnodeDOM<IOverflowingListAttrs, this>) {
+    super.oncreate(vnode);
+
+    // Watch the container rather than the row itself. The row's own width is
+    // a *result* of collapsing — observing it would mean reacting to this
+    // component's own output and never seeing the space open back up. The
+    // container tracks the viewport, and its other children (a logo, the
+    // controls opposite) can change width with no resize event of their own.
+    //
+    // Where there is no ResizeObserver the row still works: it lays out on
+    // creation and on window resizes, and only misses changes that resize a
+    // sibling without resizing the window.
+    if (typeof ResizeObserver !== 'undefined') {
+      this.observer = new ResizeObserver(() => this.recalculate());
+
+      const list = this.element as HTMLElement;
+      const container = list.parentElement?.parentElement ?? list.parentElement ?? list;
+      this.observer.observe(container);
+    }
+
+    // Sibling controls can change width without the container resizing at all
+    // — a label dropping at a breakpoint, a badge appearing. Nothing reports
+    // that, so the row also re-checks on viewport changes.
+    this.onWindowResize = () => this.recalculate();
+    window.addEventListener('resize', this.onWindowResize);
+    window.addEventListener('orientationchange', this.onWindowResize);
+
+    this.recalculate();
+  }
+
+  onupdate(vnode: Mithril.VnodeDOM<IOverflowingListAttrs, this>) {
+    super.onupdate(vnode);
+
+    this.recalculate();
+  }
+
+  onremove(vnode: Mithril.VnodeDOM<IOverflowingListAttrs, this>) {
+    super.onremove(vnode);
+
+    this.observer?.disconnect();
+
+    if (this.onWindowResize) {
+      window.removeEventListener('resize', this.onWindowResize);
+      window.removeEventListener('orientationchange', this.onWindowResize);
+    }
+  }
+
+  /**
+   * Work out how many items fit, and redraw if that has changed.
+   */
+  protected recalculate(): void {
+    const list = this.element as HTMLElement;
+    if (!list) return;
+
+    const children = Array.from(list.children) as HTMLElement[];
+    const itemCount = (this.attrs.items || []).length;
+
+    // Nothing can be measured before the row is in a document, so leave every
+    // item showing until it is.
+    if (!list.isConnected) return;
+
+    // Collapsing only makes sense while the items share a single line. In the
+    // drawer the same list is laid out as a vertical column with a whole
+    // screen height to grow into, so there is nothing to save and everything
+    // should simply render.
+    if (getComputedStyle(list).flexDirection !== 'row') {
+      if (this.visibleCount !== itemCount) {
+        this.visibleCount = itemCount;
+        m.redraw();
+      }
+      return;
+    }
+
+    // Widths are read whenever every item is on the row, which is the only
+    // time they are all measurable. Re-reading rather than trusting the first
+    // measurement matters because what an item is worth changes underneath
+    // this: a label hidden at a breakpoint, a font that finishes loading, a
+    // count appearing on a badge.
+    if (this.visibleCount === null || this.visibleCount === itemCount) {
+      const measured = children.slice(0, itemCount).map((child) => this.outerWidth(child));
+      if (measured.length === itemCount) this.itemWidths = measured;
+      // Nothing has overflowed, so the toggle is not in the DOM to measure.
+      // Reserve a conservative width for it; it is replaced with the real
+      // figure as soon as the toggle renders.
+      if (!this.toggleWidth) this.toggleWidth = 48;
+    } else {
+      const toggle = list.querySelector('.OverflowingList-toggle') as HTMLElement | null;
+      if (toggle) this.toggleWidth = this.outerWidth(toggle);
+    }
+
+    const available = this.availableWidth(list);
+    if (!available || this.itemWidths.length !== itemCount) return;
+
+    // Decided from the measured widths and the container's width — never from
+    // the row's current width, which is a consequence of the last decision and
+    // would turn this into a feedback loop.
+    const fits = countItemsThatFit(this.itemWidths, available, this.toggleWidth);
+
+    if (fits !== this.visibleCount) {
+      this.visibleCount = fits;
+      m.redraw();
+    }
+  }
+
+  /**
+   * How much room the row could occupy, rather than how much it currently
+   * does.
+   *
+   * The list sits in a flex parent that shrink-wraps its contents, so once
+   * items have been collapsed the element reports the narrower width it took
+   * *because* of the collapse. Measuring that would be circular — the row
+   * could never grow back. What matters is the gap to whatever sits beside it.
+   */
+  protected availableWidth(list: HTMLElement): number {
+    const parent = list.parentElement;
+    if (!parent) return list.clientWidth;
+
+    const container = parent.parentElement;
+    if (!container) return parent.clientWidth;
+
+    const parentBox = parent.getBoundingClientRect();
+    const containerStyle = getComputedStyle(container);
+    const containerBox = container.getBoundingClientRect();
+
+    // Start from the container's inner edge, then walk the siblings and take
+    // out whatever they occupy along with the gaps between them.
+    let free = containerBox.width - parseFloat(containerStyle.paddingLeft || '0') - parseFloat(containerStyle.paddingRight || '0');
+
+    const gap = parseFloat(containerStyle.columnGap || containerStyle.gap || '0') || 0;
+    let siblings = 0;
+
+    Array.from(container.children).forEach((child) => {
+      if (child === parent) return;
+      siblings++;
+      free -= (child as HTMLElement).getBoundingClientRect().width;
+    });
+
+    free -= gap * siblings;
+
+    // Anything the row's own parent adds around it — padding, a border, a
+    // margin — is not usable by the items themselves.
+    free -= parentBox.width - parent.clientWidth;
+
+    return Math.max(0, Math.floor(free));
+  }
+
+  protected outerWidth(el: HTMLElement): number {
+    const style = getComputedStyle(el);
+
+    return el.getBoundingClientRect().width + parseFloat(style.marginLeft || '0') + parseFloat(style.marginRight || '0');
+  }
+}

framework/core/js/src/common/components/SelectDropdown.tsx

@@ -50,6 +50,10 @@ export default class SelectDropdown<CustomAttrs extends ISelectDropdownAttrs = I
     let label = (activeChild && typeof activeChild === 'object' && 'children' in activeChild && activeChild.children) || this.attrs.defaultLabel;
 
     return [
+      // An icon is what identifies the control once there is no room for its
+      // label — without one, hiding the label leaves a caret floating on its
+      // own with nothing to say what it selects.
+      this.attrs.icon ? <Icon name={this.attrs.icon} className="Button-icon" /> : null,
       <span className="Button-label">
         <span className="Button-labelText">{label}</span>
       </span>,

framework/core/js/src/common/utils/countItemsThatFit.ts

@@ -0,0 +1,45 @@
+/**
+ * Work out how many items from the start of a row can be shown before the rest
+ * have to be collapsed into an overflow menu.
+ *
+ * Kept separate from the component that uses it so the decision can be
+ * exercised directly: it is pure arithmetic over measured widths, and the
+ * awkward cases — a boundary where an item exactly fits, a row that has to make
+ * room for the toggle it is about to show — are the ones most worth pinning
+ * down in tests.
+ *
+ * The result depends only on the arguments. Nothing here may read from the row
+ * being laid out: its width is a *consequence* of the last decision, so feeding
+ * it back in produces a layout that oscillates rather than settles.
+ *
+ * @param itemWidths Width of each item, in order, as measured while they were
+ *                   all on the row.
+ * @param available  Room the row has to lay items out in.
+ * @param toggleWidth Width of the overflow toggle, needed whenever at least one
+ *                    item is collapsed.
+ * @returns How many leading items to show. Everything after them belongs in the
+ *          overflow menu.
+ */
+export default function countItemsThatFit(itemWidths: number[], available: number, toggleWidth: number): number {
+  const itemCount = itemWidths.length;
+
+  if (itemCount === 0) return 0;
+
+  const total = itemWidths.reduce((sum, width) => sum + width, 0);
+
+  // Everything fits, so no toggle is needed and nothing is held back.
+  if (total <= available) return itemCount;
+
+  // Something has to be collapsed, which means the toggle will be shown and
+  // has to be paid for out of the same space.
+  let used = 0;
+  let fits = 0;
+
+  for (let i = 0; i < itemCount; i++) {
+    if (used + itemWidths[i] + toggleWidth > available) break;
+    used += itemWidths[i];
+    fits++;
+  }
+
+  return fits;
+}