web: Consistent Tab Panel URL Parameters (#17804)

* web: Fix tabpanel focus attempting to scroll page.

* web: Fix issues surrounding consistent tab panel parameter testing.

* web: add shared helpers.

* web: Tidy comments.

Author: GirlBossRush

web/src/elements/Tabs.ts

@@ -1,4 +1,4 @@
-import { CURRENT_CLASS, EVENT_REFRESH, ROUTE_SEPARATOR } from "#common/constants";
+import { CURRENT_CLASS, EVENT_REFRESH } from "#common/constants";
 
 import { AKElement } from "#elements/Base";
 import { getURLParams, updateURLParams } from "#elements/router/RouteMatch";
@@ -7,8 +7,7 @@ import { isFocusable } from "#elements/utils/focus";
 
 import { msg } from "@lit/localize";
 import { css, CSSResult, html, LitElement, TemplateResult } from "lit";
-import { customElement, property } from "lit/decorators.js";
-import { ifDefined } from "lit/directives/if-defined.js";
+import { customElement, property, state } from "lit/decorators.js";
 import { createRef, ref } from "lit/directives/ref.js";
 
 import PFTabs from "@patternfly/patternfly/components/Tabs/tabs.css";
@@ -20,18 +19,6 @@ export class Tabs extends AKElement {
         ...LitElement.shadowRootOptions,
         delegatesFocus: true,
     };
-
-    #focusTargetRef = createRef<HTMLSlotElement>();
-
-    @property()
-    pageIdentifier = "page";
-
-    @property()
-    currentPage?: string;
-
-    @property({ type: Boolean })
-    vertical = false;
-
     static styles: CSSResult[] = [
         PFGlobal,
         PFTabs,
@@ -55,37 +42,90 @@ export class Tabs extends AKElement {
         `,
     ];
 
-    observer: MutationObserver;
+    @property({ type: String })
+    public pageIdentifier = "page";
 
-    constructor() {
-        super();
-        this.observer = new MutationObserver(() => {
-            this.requestUpdate();
-        });
-    }
+    @property({ type: Boolean, useDefault: true })
+    public vertical = false;
+
+    @state()
+    protected activeTabName: string | null = null;
+
+    @state()
+    protected tabs: ReadonlyMap<string, Element> = new Map();
+
+    #focusTargetRef = createRef<HTMLSlotElement>();
+    #observer: MutationObserver | null = null;
+
+    #updateTabs = (): void => {
+        this.tabs = new Map(
+            Array.from(this.querySelectorAll(":scope > [slot^='page-']"), (element) => {
+                return [element.getAttribute("slot") || "", element];
+            }),
+        );
+    };
 
-    connectedCallback(): void {
+    public override connectedCallback(): void {
         super.connectedCallback();
-        this.observer.observe(this, {
+
+        this.#observer = new MutationObserver(this.#updateTabs);
+
+        this.addEventListener("focus", this.#delegateFocusListener);
+
+        if (!this.activeTabName) {
+            const params = getURLParams();
+            const tabParam = params[this.pageIdentifier];
+
+            if (
+                tabParam &&
+                typeof tabParam === "string" &&
+                this.querySelector(`[slot='${tabParam}']`)
+            ) {
+                this.activeTabName = tabParam;
+            } else {
+                this.#updateTabs();
+                this.activeTabName = this.tabs.keys().next().value || null;
+            }
+        }
+    }
+
+    public override firstUpdated(): void {
+        this.#observer?.observe(this, {
             attributes: true,
             childList: true,
             subtree: true,
         });
-
-        this.addEventListener("focus", this.#delegateFocusListener);
     }
 
-    disconnectedCallback(): void {
-        this.observer.disconnect();
+    public override disconnectedCallback(): void {
+        this.#observer?.disconnect();
         super.disconnectedCallback();
     }
 
-    onClick(slot?: string): void {
-        this.currentPage = slot;
-        const params: { [key: string]: string | undefined } = {};
-        params[this.pageIdentifier] = slot;
-        updateURLParams(params);
-        const page = this.querySelector(`[slot='${this.currentPage}']`);
+    public activateTab(nextTabName: string): void {
+        if (!nextTabName) {
+            console.warn("Cannot activate falsey tab name:", nextTabName);
+            return;
+        }
+
+        if (!this.tabs.has(nextTabName)) {
+            console.warn("Cannot activate unknown tab name:", nextTabName, this.tabs);
+            return;
+        }
+
+        const firstTab = this.tabs.keys().next().value || null;
+
+        // We avoid adding the tab parameter to the URL if it's the first tab
+        // to both reduce URL length and ensure that tests do not have to deal with
+        // unnecessary URL parameters.
+
+        updateURLParams({
+            [this.pageIdentifier]: nextTabName === firstTab ? null : nextTabName,
+        });
+
+        this.activeTabName = nextTabName;
+
+        const page = this.querySelector(`[slot='${this.activeTabName}']`);
         if (!page) return;
 
         page.dispatchEvent(new CustomEvent(EVENT_REFRESH));
@@ -103,59 +143,49 @@ export class Tabs extends AKElement {
 
         // We don't want to refocus if the user is tabbing between elements inside the tabpanel.
         if (focusableElement && event.relatedTarget !== focusableElement) {
-            focusableElement.focus();
+            focusableElement.focus({
+                preventScroll: true,
+            });
         }
     };
 
-    renderTab(page: Element): TemplateResult {
-        const slot = page.attributes.getNamedItem("slot")?.value;
-        return html` <li class="pf-c-tabs__item ${slot === this.currentPage ? CURRENT_CLASS : ""}">
+    renderTab(slotName: string, tabPanel: Element): TemplateResult {
+        return html` <li
+            class="pf-c-tabs__item ${slotName === this.activeTabName ? CURRENT_CLASS : ""}"
+        >
             <button
                 type="button"
                 role="tab"
-                id=${`${slot}-tab`}
-                aria-selected=${slot === this.currentPage ? "true" : "false"}
-                aria-controls=${ifPresent(slot)}
+                id=${`${slotName}-tab`}
+                aria-selected=${slotName === this.activeTabName ? "true" : "false"}
+                aria-controls=${ifPresent(slotName)}
                 class="pf-c-tabs__link"
-                @click=${() => this.onClick(slot)}
+                @click=${() => this.activateTab(slotName)}
             >
-                <span class="pf-c-tabs__item-text"> ${page.getAttribute("aria-label")}</span>
+                <span class="pf-c-tabs__item-text"> ${tabPanel.getAttribute("aria-label")}</span>
             </button>
         </li>`;
     }
 
     render(): TemplateResult {
-        const pages = Array.from(this.querySelectorAll(":scope > [slot^='page-']"));
-        if (window.location.hash.includes(ROUTE_SEPARATOR)) {
-            const params = getURLParams();
-            if (
-                this.pageIdentifier in params &&
-                !this.currentPage &&
-                this.querySelector(`[slot='${params[this.pageIdentifier]}']`) !== null
-            ) {
-                // To update the URL to match with the current slot
-                this.onClick(params[this.pageIdentifier] as string);
-            }
-        }
-        if (!this.currentPage) {
-            if (pages.length < 1) {
-                return html`<h1>${msg("no tabs defined")}</h1>`;
-            }
-            const wantedPage = pages[0].attributes.getNamedItem("slot")?.value;
-            this.onClick(wantedPage);
+        if (!this.tabs.size) {
+            return html`<h1>${msg("no tabs defined")}</h1>`;
         }
+
         return html`<div class="pf-c-tabs ${this.vertical ? "pf-m-vertical pf-m-box" : ""}">
                 <ul
                     class="pf-c-tabs__list"
                     role="tablist"
                     aria-orientation=${this.vertical ? "vertical" : "horizontal"}
                     aria-label=${ifPresent(this.ariaLabel)}
                 >
-                    ${pages.map((page) => this.renderTab(page))}
+                    ${Array.from(this.tabs, ([slotName, tabPanel]) =>
+                        this.renderTab(slotName, tabPanel),
+                    )}
                 </ul>
             </div>
             <slot name="header"></slot>
-            <slot ${ref(this.#focusTargetRef)} name="${ifDefined(this.currentPage)}"></slot>`;
+            <slot ${ref(this.#focusTargetRef)} name=${ifPresent(this.activeTabName)}></slot>`;
     }
 }
 

web/src/elements/router/RouteMatch.ts

@@ -1,6 +1,7 @@
 import { ROUTE_SEPARATOR } from "#common/constants";
 
 import { Route } from "#elements/router/Route";
+import { RouteParameterRecord } from "#elements/router/shared";
 
 import { TemplateResult } from "lit";
 
@@ -49,10 +50,10 @@ export function getURLParam<T>(key: string, fallback: T): T {
     return fallback;
 }
 
-export function getURLParams(): { [key: string]: unknown } {
+export function getURLParams(): RouteParameterRecord {
     const params = {};
     if (window.location.hash.includes(ROUTE_SEPARATOR)) {
-        const urlParts = window.location.hash.slice(1, Infinity).split(ROUTE_SEPARATOR, 2);
+        const urlParts = window.location.hash.slice(1).split(ROUTE_SEPARATOR, 2);
         const rawParams = decodeURIComponent(urlParts[1]);
         try {
             return JSON.parse(rawParams);
@@ -63,21 +64,43 @@ export function getURLParams(): { [key: string]: unknown } {
     return params;
 }
 
-export function setURLParams(params: { [key: string]: unknown }, replace = true): void {
-    const paramsString = JSON.stringify(params);
-    const currentUrl = window.location.hash.slice(1, Infinity).split(ROUTE_SEPARATOR)[0];
-    const newUrl = `#${currentUrl};${encodeURIComponent(paramsString)}`;
+/**
+ * Serialize route parameters to a JSON string, removing empty values.
+ *
+ * @param params The route parameters to serialize.
+ */
+export function prepareURLParams(params: RouteParameterRecord): RouteParameterRecord {
+    const preparedParams: RouteParameterRecord = {};
+    for (const [key, value] of Object.entries(params)) {
+        if (value !== null && value !== undefined && value !== "") {
+            preparedParams[key] = value;
+        }
+    }
+    return preparedParams;
+}
+
+export function serializeURLParams(params: RouteParameterRecord): string {
+    const preparedParams = prepareURLParams(params);
+
+    return Object.keys(preparedParams).length === 0 ? "" : JSON.stringify(preparedParams);
+}
+
+export function setURLParams(params: RouteParameterRecord, replace = true): void {
+    const [currentHash] = window.location.hash.slice(1).split(ROUTE_SEPARATOR);
+    let nextHash = "#" + currentHash;
+    const preparedParams = prepareURLParams(params);
+
+    if (Object.keys(preparedParams).length) {
+        nextHash += ROUTE_SEPARATOR + encodeURIComponent(JSON.stringify(preparedParams));
+    }
+
     if (replace) {
-        history.replaceState(undefined, "", newUrl);
+        history.replaceState(undefined, "", nextHash);
     } else {
-        history.pushState(undefined, "", newUrl);
+        history.pushState(undefined, "", nextHash);
     }
 }
 
-export function updateURLParams(params: { [key: string]: unknown }, replace = true): void {
-    const currentParams = getURLParams();
-    for (const key in params) {
-        currentParams[key] = params[key] as string;
-    }
-    setURLParams(currentParams, replace);
+export function updateURLParams(params: RouteParameterRecord, replace = true): void {
+    setURLParams({ ...getURLParams(), ...params }, replace);
 }

web/src/elements/router/shared.ts

@@ -0,0 +1,6 @@
+/**
+ * @file Common types for routing.
+ */
+
+export type PrimitiveRouteParameter = string | number | boolean | null | undefined;
+export type RouteParameterRecord = { [key: string]: PrimitiveRouteParameter };

web/src/elements/router/utils.ts

@@ -14,7 +14,7 @@ export type RouteInterfaceName = "user" | "admin" | "flow" | "unknown";
 /**
  * Read the current interface route parameter from the URL.
  *
- * @param location - The location object to read the pathname from. Defaults to `window.location`.
+ * @param location The location object to read the pathname from. Defaults to `window.location`.
  * @returns The name of the current interface, or "unknown" if not found.
  *
  * @category Routing
@@ -51,7 +51,7 @@ export function isUserRoute(location: Pick<URL, "pathname"> = window.location):
  * The input is converted to lowercase and non-alphanumeric characters are
  * replaced with a hyphen. Trailing whitespace and hyphens are removed.
  *
- * @param input - The input string to format.
+ * @param input The input string to format.
  *
  * @category Routing
  *
@@ -68,7 +68,7 @@ export function formatSlug(input: string): string {
 /**
  * Predicate to determine if the input is a valid route slug.
  *
- * @param input - The input string to check.
+ * @param input The input string to check.
  */
 export function isSlug(input: string): boolean {
     return kebabCase(input) === input;

web/src/elements/table/Table.ts

@@ -148,7 +148,7 @@ export abstract class Table<T extends object>
     @property({ attribute: false })
     public data: PaginatedResponse<T> | null = null;
 
-    @property({ type: Number })
+    @property({ type: Number, useDefault: true })
     public page = getURLParam(this.#pageParam, 1);
 
     /**
@@ -256,7 +256,7 @@ export abstract class Table<T extends object>
     protected willUpdate(changedProperties: PropertyValues<this>): void {
         if (changedProperties.has("page")) {
             updateURLParams({
-                [this.#pageParam]: this.page,
+                [this.#pageParam]: this.page === 1 ? null : this.page,
             });
         }
         if (changedProperties.has("search")) {