From 01b81624a399c6f41775a1e51216cfe04877e6b9 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 2 Oct 2026 16:44:07 +0800 Subject: [PATCH 01/15] fix(change_request): render wizard search results as keyboard-reachable options (#580) The Create Change Request wizard renders its registrant search results server-side as plain table rows with a mouse click handler, and a pager made of tags without href. Neither can take keyboard focus, so a keyboard or screen-reader user could type a search but never pick a result (WCAG 2.1.1, 4.1.2). Rows are now options of a listbox (role, roving tabindex, aria-label with name and type), the pager is made of real buttons whose out-of-range side carries a disabled attribute instead of muted styling, the decorative type icons are hidden from assistive technology, and the range summary / empty-result message carry a status class the widget mirrors into a live region. Nothing changes visually. Tests pin the rendered semantics and the two bridge behaviours the widget relies on (selected-partner onchange, page re-render and clamp), none of which were covered before. --- spp_change_request_v2/tests/__init__.py | 1 + .../tests/test_create_wizard_search.py | 133 ++++++++++++++++++ .../wizards/create_wizard.py | 53 +++++-- 3 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 spp_change_request_v2/tests/test_create_wizard_search.py diff --git a/spp_change_request_v2/tests/__init__.py b/spp_change_request_v2/tests/__init__.py index 0a0e4a967..392178942 100644 --- a/spp_change_request_v2/tests/__init__.py +++ b/spp_change_request_v2/tests/__init__.py @@ -38,3 +38,4 @@ from . import test_field_mapping_transform from . import test_routing_field from . import test_birthdate_mixin +from . import test_create_wizard_search diff --git a/spp_change_request_v2/tests/test_create_wizard_search.py b/spp_change_request_v2/tests/test_create_wizard_search.py new file mode 100644 index 000000000..e84cedfda --- /dev/null +++ b/spp_change_request_v2/tests/test_create_wizard_search.py @@ -0,0 +1,133 @@ +"""Tests for the registrant search in the Change Request creation wizard. + +The wizard renders its search results server-side as HTML and the browser +widget only wires up selection and paging. These tests pin down the markup +that keyboard users and assistive technology depend on (focusable rows with +a role and a name, a real button pager, status text for a live region) and +the bridge fields the widget writes. +""" + +from lxml import html as lxml_html + +from odoo.tests import tagged + +from .test_change_request import TestChangeRequestBase + +PREFIX = "A11Y Searchable" + + +@tagged("post_install", "-at_install", "cr_ux") +class TestCreateWizardSearch(TestChangeRequestBase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.individuals = cls.partner_model.create( + [{"name": f"{PREFIX} {index:02d}", "is_registrant": True, "is_group": False} for index in range(1, 13)] + ) + cls.search_group = cls.partner_model.create( + {"name": f"{PREFIX} Group", "is_registrant": True, "is_group": True} + ) + cls.wizard_model = cls.env["spp.cr.create.wizard"] + + def _wizard(self): + return self.wizard_model.create({"request_type_id": self.cr_type_edit_individual.id}) + + def _render(self, wizard, text, page=0): + wizard.search_text = text + wizard._onchange_search_text() + if page: + wizard._search_page = page + wizard._onchange_search_page() + return lxml_html.fragment_fromstring(str(wizard.search_results_html), create_parent="div") + + @staticmethod + def _rows(doc): + return doc.xpath('.//tr[contains(@class, "o_cr_search_result")]') + + @staticmethod + def _status_text(doc): + nodes = doc.xpath('.//*[contains(@class, "o_cr_search_status")]') + return nodes[0].text_content().strip() if nodes else None + + def test_rows_are_focusable_options(self): + """Rows are options of a listbox, one of them in the Tab order, each with an accessible name.""" + doc = self._render(self._wizard(), PREFIX) + + listbox = doc.xpath('.//tbody[@role="listbox"]') + self.assertEqual(len(listbox), 1) + self.assertTrue(listbox[0].get("aria-label")) + + rows = self._rows(doc) + self.assertEqual(len(rows), 10) + self.assertEqual([row.get("role") for row in rows], ["option"] * 10) + self.assertEqual([row.get("tabindex") for row in rows], ["0"] + ["-1"] * 9) + for row in rows: + self.assertEqual(row.get("aria-label"), f"{row.get('data-partner-name')}, Individual") + + def test_type_icon_is_hidden_from_assistive_tech(self): + """Decorative type icons are not announced.""" + doc = self._render(self._wizard(), PREFIX) + icons = doc.xpath('.//i[contains(@class, "fa")]') + self.assertEqual(len(icons), 10) + for icon in icons: + self.assertEqual(icon.get("aria-hidden"), "true") + + def test_pager_renders_buttons_with_disabled_edges(self): + """Previous/Next are real buttons; the edge of the range is disabled rather than merely styled.""" + wizard = self._wizard() + + doc = self._render(wizard, PREFIX) + prev = doc.xpath('.//button[contains(@class, "o_cr_page_prev")]')[0] + nxt = doc.xpath('.//button[contains(@class, "o_cr_page_next")]')[0] + self.assertEqual((prev.get("type"), nxt.get("type")), ("button", "button")) + self.assertIsNotNone(prev.get("disabled")) + self.assertIsNone(nxt.get("disabled")) + self.assertEqual((prev.get("data-page"), nxt.get("data-page")), ("-1", "1")) + + doc = self._render(wizard, PREFIX, page=1) + prev = doc.xpath('.//button[contains(@class, "o_cr_page_prev")]')[0] + nxt = doc.xpath('.//button[contains(@class, "o_cr_page_next")]')[0] + self.assertIsNone(prev.get("disabled")) + self.assertIsNotNone(nxt.get("disabled")) + self.assertEqual((prev.get("data-page"), nxt.get("data-page")), ("0", "2")) + + def test_status_text_is_marked_for_live_region(self): + """The range summary and the empty-result message carry the status marker the widget mirrors.""" + wizard = self._wizard() + self.assertEqual(self._status_text(self._render(wizard, PREFIX)), "1-10 of 12") + self.assertEqual(self._status_text(self._render(wizard, PREFIX, page=1)), "11-12 of 12") + self.assertEqual(self._status_text(self._render(wizard, f"{PREFIX} nothing matches")), "No registrants found.") + + def test_aria_label_escapes_name(self): + """A name containing markup is escaped in the accessible name as it is in the cell.""" + self.partner_model.create({"name": f"{PREFIX} bold", "is_registrant": True, "is_group": False}) + wizard = self._wizard() + wizard.search_text = f"{PREFIX} " + wizard._onchange_search_text() + raw = str(wizard.search_results_html) + + self.assertIn(f'aria-label="{PREFIX} <b>bold</b>, Individual"', raw) + self.assertNotIn("bold", raw) + + def test_selected_partner_bridge_sets_registrant(self): + """The integer the widget writes on Enter/click becomes the registrant.""" + wizard = self._wizard() + chosen = self.individuals[3] + wizard._selected_partner_id = chosen.id + wizard._onchange_selected_partner() + self.assertEqual(wizard.registrant_id, chosen) + + def test_page_change_rerenders_and_clamps(self): + """Paging shows the remaining rows, never the group, and an out-of-range page falls back to the last one.""" + wizard = self._wizard() + first_page = {row.get("data-partner-name") for row in self._rows(self._render(wizard, PREFIX))} + second_page = {row.get("data-partner-name") for row in self._rows(self._render(wizard, PREFIX, page=1))} + + self.assertEqual(len(first_page), 10) + self.assertEqual(len(second_page), 2) + self.assertEqual(first_page | second_page, set(self.individuals.mapped("name"))) + self.assertNotIn(self.search_group.name, first_page | second_page) + + doc = self._render(wizard, PREFIX, page=99) + self.assertEqual(len(self._rows(doc)), 2) + self.assertEqual(self._status_text(doc), "11-12 of 12") diff --git a/spp_change_request_v2/wizards/create_wizard.py b/spp_change_request_v2/wizards/create_wizard.py index 2f1c25438..9155abefa 100644 --- a/spp_change_request_v2/wizards/create_wizard.py +++ b/spp_change_request_v2/wizards/create_wizard.py @@ -280,7 +280,7 @@ def _render_search_results(self): total = self.env["res.partner"].search_count(search_domain) if not total: - self.search_results_html = Markup("

No registrants found.

") + self.search_results_html = Markup("

No registrants found.

") return page = self._search_page or 0 @@ -291,42 +291,65 @@ def _render_search_results(self): offset = page * page_size partners = self.env["res.partner"].search(search_domain, limit=page_size, offset=offset) + # Each row is an option of a listbox so keyboard and screen-reader + # users can reach and pick it; the widget handles the keys. Only the + # first row is in the Tab order (roving tabindex), the arrow keys move + # between rows. The aria-label repeats what the two cells show because + # an option's children are presentational to assistive technology. rows = [] - for p in partners: - ptype = ' Group' if p.is_group else ' Individual' + for index, p in enumerate(partners): + type_label, type_icon = ("Group", "fa-users") if p.is_group else ("Individual", "fa-user") + name = escape(p.name or "") rows.append( Markup( '' "{}" - "{}" + ' {}' ).format( + "0" if index == 0 else "-1", + name, + type_label, p.id, - escape(p.name or ""), - escape(p.name or ""), - Markup(ptype), + name, + name, + type_icon, + type_label, ) ) table = Markup( '' "" - "{}
NameType
" + '{}' ).format(Markup("").join(rows)) - # Pagination header + # Pagination header. Previous/Next are real buttons so they are + # focusable and respond to Enter/Space natively; a real `disabled` + # attribute (not just muted styling) takes the edge out of the Tab order. + # The range summary carries o_cr_search_status so the widget can mirror + # it into a live region. start = offset + 1 end = min(offset + page_size, total) - prev_cls = "text-muted" if page == 0 else "o_cr_page_prev" - next_cls = "text-muted" if page >= max_page else "o_cr_page_next" pagination = Markup( '
' - '{}-{} of {}' + '{}-{} of {}' "
" - '← Previous' - 'Next →' + '' + '' "
" - ).format(start, end, total, prev_cls, page - 1, next_cls, page + 1) + ).format( + start, + end, + total, + page - 1, + " disabled" if page == 0 else "", + page + 1, + " disabled" if page >= max_page else "", + ) self.search_results_html = pagination + table From 1a9611aa996314ab3663f4f1db139071a67e9ce4 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 2 Oct 2026 16:44:07 +0800 Subject: [PATCH 02/15] fix(change_request): keyboard handling, focus management and live region for the search results widget (#580) Enter/Space select the focused row, ArrowUp/ArrowDown/Home/End move between rows with a roving tabindex, and clicks keep working through one delegated listener instead of per-row handlers re-attached after every patch. Every search or page change re-renders the whole results blob, which destroys the focused node, so focus is put back on the pager button that was pressed (or the other one, or the first row) once the form has re-rendered; selecting a row hides the results block and unmounts the widget, so focus is handed to the "Change Registrant" button instead of dropping to . A visually-hidden live region outside the re-rendered blob announces the result range and the empty-result message. The debounced search input now carries the field id so its "Search Registrant" label names it rather than the placeholder, and a focus ring is added for rows since the table-hover shading swallows the default one. --- .../static/src/css/cr_search_results.css | 6 + .../static/src/js/cr_search_results_field.js | 199 +++++++++++++++--- .../src/xml/cr_search_results_field.xml | 15 +- .../static/src/xml/search_delay_field.xml | 1 + 4 files changed, 187 insertions(+), 34 deletions(-) diff --git a/spp_change_request_v2/static/src/css/cr_search_results.css b/spp_change_request_v2/static/src/css/cr_search_results.css index 166021a0c..7c1b5c4af 100644 --- a/spp_change_request_v2/static/src/css/cr_search_results.css +++ b/spp_change_request_v2/static/src/css/cr_search_results.css @@ -5,3 +5,9 @@ max-width: 100% !important; display: block !important; } + +/* Keyboard focus on a result row must stay visible over the table-hover shading */ +.o_field_cr_search_results tr.o_cr_search_result:focus-visible { + outline: 2px solid var(--bs-primary); + outline-offset: -2px; +} diff --git a/spp_change_request_v2/static/src/js/cr_search_results_field.js b/spp_change_request_v2/static/src/js/cr_search_results_field.js index 024c96574..2c46c91c7 100644 --- a/spp_change_request_v2/static/src/js/cr_search_results_field.js +++ b/spp_change_request_v2/static/src/js/cr_search_results_field.js @@ -5,11 +5,20 @@ import {_t} from "@web/core/l10n/translation"; import {registry} from "@web/core/registry"; import {standardFieldProps} from "@web/views/fields/standard_field_props"; +const ROW_SELECTOR = ".o_cr_search_result"; +const PAGER_SELECTOR = ".o_cr_page_prev, .o_cr_page_next"; +const STATUS_SELECTOR = ".o_cr_search_status"; +const CLEAR_BUTTON_SELECTOR = "button[name='action_clear_registrant']"; +// Upper bound on waiting for the form to re-render after a record update. +const RENDER_WAIT_MS = 500; + /** - * Custom widget that renders HTML search results and handles row clicks. - * When a row with class "o_cr_search_result" is clicked, it writes the - * partner ID to the _selected_partner_id bridge field, which triggers - * a server-side onchange to set registrant_id. + * Custom widget that renders HTML search results and handles row selection + * and paging by mouse or keyboard. The rows are options of a listbox: Enter + * or Space selects the focused row, ArrowUp/ArrowDown/Home/End move between + * rows. Selecting a row writes the partner ID to the _selected_partner_id + * bridge field, which triggers a server-side onchange to set registrant_id; + * paging writes _search_page the same way. */ export class CrSearchResultsField extends Component { static template = "spp_change_request_v2.CrSearchResultsField"; @@ -17,39 +26,173 @@ export class CrSearchResultsField extends Component { setup() { this.containerRef = useRef("container"); - onMounted(() => this._attachClickHandler()); - onPatched(() => this._attachClickHandler()); + this.statusRef = useRef("status"); + this.resolveNextPatch = null; + onMounted(() => { + // One delegated listener each: the server re-renders the whole + // results blob on every search or page change, so per-row handlers + // would have to be re-attached after each patch. + this.containerRef.el.addEventListener("click", (ev) => this._onClick(ev)); + this.containerRef.el.addEventListener("keydown", (ev) => + this._onKeydown(ev) + ); + this._announceStatus(); + }); + onPatched(() => { + this._announceStatus(); + if (this.resolveNextPatch) { + const resolve = this.resolveNextPatch; + this.resolveNextPatch = null; + resolve(); + } + }); } get htmlContent() { return this.props.record.data[this.props.name] || ""; } - _attachClickHandler() { - const el = this.containerRef.el; - if (!el) return; - // Row selection - el.querySelectorAll(".o_cr_search_result").forEach((row) => { - row.onclick = (ev) => { + get rows() { + return [...this.containerRef.el.querySelectorAll(ROW_SELECTOR)]; + } + + _onClick(ev) { + const row = ev.target.closest(ROW_SELECTOR); + if (row) { + ev.preventDefault(); + ev.stopPropagation(); + this._selectRow(row); + return; + } + const pager = ev.target.closest(PAGER_SELECTOR); + if (pager && !pager.disabled) { + ev.preventDefault(); + ev.stopPropagation(); + this._goToPage(pager); + } + } + + _onKeydown(ev) { + // The pager buttons are native buttons: Enter and Space already click them. + const row = ev.target.closest(ROW_SELECTOR); + if (!row) { + return; + } + const rows = this.rows; + const index = rows.indexOf(row); + let target = null; + switch (ev.key) { + case "Enter": + case " ": ev.preventDefault(); - ev.stopPropagation(); - const partnerId = parseInt(row.dataset.partnerId, 10); - if (partnerId) { - this.props.record.update({_selected_partner_id: partnerId}); - } - }; + this._selectRow(row); + return; + case "ArrowDown": + target = rows[Math.min(index + 1, rows.length - 1)]; + break; + case "ArrowUp": + target = rows[Math.max(index - 1, 0)]; + break; + case "Home": + target = rows[0]; + break; + case "End": + target = rows[rows.length - 1]; + break; + default: + return; + } + ev.preventDefault(); + this._focusRow(target, rows); + } + + /** + * Roving tabindex: the list is a single Tab stop, and the row that holds + * focus is the one Tab returns to. + */ + _focusRow(row, rows) { + for (const other of rows) { + other.tabIndex = -1; + } + row.tabIndex = 0; + row.focus(); + } + + async _selectRow(row) { + const partnerId = parseInt(row.dataset.partnerId, 10); + if (!partnerId) { + return; + } + // Setting the registrant hides the results block, which unmounts this + // widget, so focus is handed to the button that brings the search back. + const formEl = this.containerRef.el.closest(".o_form_view"); + await this.props.record.update({_selected_partner_id: partnerId}); + await this._focusWhenRendered(formEl, [CLEAR_BUTTON_SELECTOR]); + } + + async _goToPage(button) { + const page = parseInt(button.dataset.page, 10); + if (isNaN(page) || page < 0) { + return; + } + const pressed = button.classList.contains("o_cr_page_next") + ? ".o_cr_page_next" + : ".o_cr_page_prev"; + const other = + pressed === ".o_cr_page_next" ? ".o_cr_page_prev" : ".o_cr_page_next"; + const rendered = this._nextPatch(); + await this.props.record.update({_search_page: page}); + await rendered; + // Keep focus on the button that was pressed so it can be pressed again; + // on the last page it is disabled, so fall back to the other one, then + // to the first row. + await this._focusWhenRendered(this.containerRef.el, [ + pressed, + other, + ROW_SELECTOR, + ]); + } + + _nextPatch() { + return new Promise((resolve) => { + this.resolveNextPatch = resolve; + setTimeout(resolve, RENDER_WAIT_MS); }); - // Pagination - el.querySelectorAll(".o_cr_page_prev, .o_cr_page_next").forEach((link) => { - link.onclick = (ev) => { - ev.preventDefault(); - ev.stopPropagation(); - const page = parseInt(link.dataset.page); - if (!isNaN(page) && page >= 0) { - this.props.record.update({_search_page: page}); + } + + /** + * Focus the first enabled element matching one of the selectors, polling a + * few animation frames because the element may only appear once the form + * has re-rendered with the onchange result. + */ + async _focusWhenRendered(root, selectors) { + if (!root) { + return; + } + const deadline = Date.now() + RENDER_WAIT_MS; + while (Date.now() < deadline) { + for (const selector of selectors) { + const target = root.querySelector(selector); + if (target && !target.disabled) { + target.focus(); + return; } - }; - }); + } + await new Promise(requestAnimationFrame); + } + } + + /** + * Mirror the range summary ("1-10 of 23") or the empty-result message into + * a live region that outlives the re-rendered blob, so screen readers hear + * the outcome of a search or a page change. + */ + _announceStatus() { + const status = this.containerRef.el.querySelector(STATUS_SELECTOR); + const text = status ? status.textContent.trim() : ""; + if (this.statusRef.el && this.statusRef.el.textContent !== text) { + this.statusRef.el.textContent = text; + } } } diff --git a/spp_change_request_v2/static/src/xml/cr_search_results_field.xml b/spp_change_request_v2/static/src/xml/cr_search_results_field.xml index b4ea3f52c..3b8f823d9 100644 --- a/spp_change_request_v2/static/src/xml/cr_search_results_field.xml +++ b/spp_change_request_v2/static/src/xml/cr_search_results_field.xml @@ -2,12 +2,15 @@ -
+
+
" ).format( start, From 9fb14bd1d4dd8017dbe934b20ef2c223c8b76446 Mon Sep 17 00:00:00 2001 From: Edwin Gonzales Date: Fri, 2 Oct 2026 17:01:45 +0800 Subject: [PATCH 06/15] fix(change_request): re-render-driven focus restore, form-hosted live region and hotkey hardening (#580) Review round. Focus after a page change was restored after a 500 ms timer that started before the onchange request was sent, so on a slow link it focused the old pager button just before the re-render removed it. The focus is now applied in onPatched when the results HTML actually changed, with a guarded fallback for an unchanged answer (page clamped to the last one). The live region moved out of the widget into the wizard form, where it exists before the first results arrive: a region inserted together with its text is not announced. The widget clears it before writing so an unchanged range text is announced again. Keys with Alt/Ctrl/Meta are left to the browser and to Odoo's hotkeys (Ctrl+Enter is the dialog's footer-button hotkey) and handled keys stop propagating. The roving tabindex follows focus that arrives by mouse or from assistive technology (focusin), not only the arrow keys. The focus ring did not paint at all: --bs-primary does not exist in Odoo 19's backend (Bootstrap is compiled without the prefix), which made the whole declaration invalid. It now uses --primary with a literal fallback, the cells carry the ring as a box-shadow because outlines on are unreliable under border-collapse, and the pager buttons get the same ring. The search box also gets autocomplete="off". --- .../static/src/css/cr_search_results.css | 21 ++- .../static/src/js/cr_search_results_field.js | 134 +++++++++++------- .../src/xml/cr_search_results_field.xml | 15 +- .../static/src/xml/search_delay_field.xml | 1 + .../views/create_wizard_views.xml | 11 ++ 5 files changed, 120 insertions(+), 62 deletions(-) diff --git a/spp_change_request_v2/static/src/css/cr_search_results.css b/spp_change_request_v2/static/src/css/cr_search_results.css index 7c1b5c4af..ed5996f59 100644 --- a/spp_change_request_v2/static/src/css/cr_search_results.css +++ b/spp_change_request_v2/static/src/css/cr_search_results.css @@ -6,8 +6,25 @@ display: block !important; } -/* Keyboard focus on a result row must stay visible over the table-hover shading */ +/* Keyboard focus on a result row must stay visible over the table-hover shading. + Odoo 19 compiles Bootstrap without the "bs-" prefix, so the theme colour is + --primary; the literal is the fallback for a theme that does not expose it. */ .o_field_cr_search_results tr.o_cr_search_result:focus-visible { - outline: 2px solid var(--bs-primary); + outline: 2px solid var(--primary, #714b67); outline-offset: -2px; } + +/* Outlines on are not painted reliably under border-collapse, so the cells carry + the ring too */ +.o_field_cr_search_results tr.o_cr_search_result:focus-visible > td { + box-shadow: + inset 0 2px 0 var(--primary, #714b67), + inset 0 -2px 0 var(--primary, #714b67); +} + +/* Link-styled pager buttons: the backend theme can suppress the default button ring */ +.o_field_cr_search_results .o_cr_page_prev:focus-visible, +.o_field_cr_search_results .o_cr_page_next:focus-visible { + outline: 2px solid var(--primary, #714b67); + outline-offset: 2px; +} diff --git a/spp_change_request_v2/static/src/js/cr_search_results_field.js b/spp_change_request_v2/static/src/js/cr_search_results_field.js index 2c46c91c7..08ae01b47 100644 --- a/spp_change_request_v2/static/src/js/cr_search_results_field.js +++ b/spp_change_request_v2/static/src/js/cr_search_results_field.js @@ -8,9 +8,12 @@ import {standardFieldProps} from "@web/views/fields/standard_field_props"; const ROW_SELECTOR = ".o_cr_search_result"; const PAGER_SELECTOR = ".o_cr_page_prev, .o_cr_page_next"; const STATUS_SELECTOR = ".o_cr_search_status"; +// Live region hosted by the wizard form, outside the results block, so it +// already exists when the first results arrive (see create_wizard_views.xml). +const LIVE_REGION_SELECTOR = ".o_cr_search_live"; const CLEAR_BUTTON_SELECTOR = "button[name='action_clear_registrant']"; // Upper bound on waiting for the form to re-render after a record update. -const RENDER_WAIT_MS = 500; +const RENDER_WAIT_MS = 2000; /** * Custom widget that renders HTML search results and handles row selection @@ -26,25 +29,30 @@ export class CrSearchResultsField extends Component { setup() { this.containerRef = useRef("container"); - this.statusRef = useRef("status"); - this.resolveNextPatch = null; + // HTML as last rendered, to tell a re-render with new results from + // any other patch of the form. + this.renderedHtml = null; + // Selectors to focus, in order of preference, once the results have + // been re-rendered after a page change. + this.pendingFocus = null; onMounted(() => { // One delegated listener each: the server re-renders the whole // results blob on every search or page change, so per-row handlers // would have to be re-attached after each patch. - this.containerRef.el.addEventListener("click", (ev) => this._onClick(ev)); - this.containerRef.el.addEventListener("keydown", (ev) => - this._onKeydown(ev) - ); + const el = this.containerRef.el; + el.addEventListener("click", (ev) => this._onClick(ev)); + el.addEventListener("keydown", (ev) => this._onKeydown(ev)); + el.addEventListener("focusin", (ev) => this._onFocusin(ev)); + this.renderedHtml = this.htmlContent; this._announceStatus(); }); onPatched(() => { - this._announceStatus(); - if (this.resolveNextPatch) { - const resolve = this.resolveNextPatch; - this.resolveNextPatch = null; - resolve(); + if (this.htmlContent === this.renderedHtml) { + return; } + this.renderedHtml = this.htmlContent; + this._announceStatus(); + this._applyPendingFocus(); }); } @@ -73,9 +81,11 @@ export class CrSearchResultsField extends Component { } _onKeydown(ev) { - // The pager buttons are native buttons: Enter and Space already click them. + // The pager buttons are native buttons: Enter and Space already click + // them. Keys with a modifier belong to the browser or to Odoo's hotkeys + // (the dialog binds Ctrl+Enter to its first footer button). const row = ev.target.closest(ROW_SELECTOR); - if (!row) { + if (!row || ev.altKey || ev.ctrlKey || ev.metaKey) { return; } const rows = this.rows; @@ -85,6 +95,7 @@ export class CrSearchResultsField extends Component { case "Enter": case " ": ev.preventDefault(); + ev.stopPropagation(); this._selectRow(row); return; case "ArrowDown": @@ -103,19 +114,23 @@ export class CrSearchResultsField extends Component { return; } ev.preventDefault(); - this._focusRow(target, rows); + ev.stopPropagation(); + target.focus(); } /** * Roving tabindex: the list is a single Tab stop, and the row that holds - * focus is the one Tab returns to. + * focus is the one Tab returns to. Handled on focusin so it also covers + * focus that arrives by mouse or from assistive technology. */ - _focusRow(row, rows) { - for (const other of rows) { - other.tabIndex = -1; + _onFocusin(ev) { + const row = ev.target.closest(ROW_SELECTOR); + if (!row) { + return; + } + for (const other of this.rows) { + other.tabIndex = other === row ? 0 : -1; } - row.tabIndex = 0; - row.focus(); } async _selectRow(row) { @@ -127,7 +142,7 @@ export class CrSearchResultsField extends Component { // widget, so focus is handed to the button that brings the search back. const formEl = this.containerRef.el.closest(".o_form_view"); await this.props.record.update({_selected_partner_id: partnerId}); - await this._focusWhenRendered(formEl, [CLEAR_BUTTON_SELECTOR]); + await this._focusWhenRendered(formEl, CLEAR_BUTTON_SELECTOR); } async _goToPage(button) { @@ -135,48 +150,58 @@ export class CrSearchResultsField extends Component { if (isNaN(page) || page < 0) { return; } + // Keep focus on the button that was pressed so it can be pressed again; + // on the last page it is disabled, so fall back to the other one, then + // to the first row. The focus is applied by onPatched once the new + // results are in the DOM, however long the onchange takes. const pressed = button.classList.contains("o_cr_page_next") ? ".o_cr_page_next" : ".o_cr_page_prev"; const other = pressed === ".o_cr_page_next" ? ".o_cr_page_prev" : ".o_cr_page_next"; - const rendered = this._nextPatch(); + this.pendingFocus = [pressed, other, ROW_SELECTOR]; await this.props.record.update({_search_page: page}); - await rendered; - // Keep focus on the button that was pressed so it can be pressed again; - // on the last page it is disabled, so fall back to the other one, then - // to the first row. - await this._focusWhenRendered(this.containerRef.el, [ - pressed, - other, - ROW_SELECTOR, - ]); + // The server may answer with the same HTML (page clamped to the last + // one), in which case no patch with new results ever comes. Only then + // is the focus applied here; while a re-render is still due, onPatched + // owns it, otherwise the old button would be focused and then removed. + await new Promise(requestAnimationFrame); + if (this.htmlContent === this.renderedHtml) { + this._applyPendingFocus(); + } } - _nextPatch() { - return new Promise((resolve) => { - this.resolveNextPatch = resolve; - setTimeout(resolve, RENDER_WAIT_MS); - }); + _applyPendingFocus() { + const selectors = this.pendingFocus; + this.pendingFocus = null; + const el = this.containerRef.el; + if (!selectors || !el) { + return; + } + for (const selector of selectors) { + const target = el.querySelector(selector); + if (target && !target.disabled) { + target.focus(); + return; + } + } } /** - * Focus the first enabled element matching one of the selectors, polling a - * few animation frames because the element may only appear once the form - * has re-rendered with the onchange result. + * Focus the element matching the selector, polling animation frames + * because it only appears once the form has re-rendered with the onchange + * result. */ - async _focusWhenRendered(root, selectors) { + async _focusWhenRendered(root, selector) { if (!root) { return; } const deadline = Date.now() + RENDER_WAIT_MS; while (Date.now() < deadline) { - for (const selector of selectors) { - const target = root.querySelector(selector); - if (target && !target.disabled) { - target.focus(); - return; - } + const target = root.querySelector(selector); + if (target) { + target.focus(); + return; } await new Promise(requestAnimationFrame); } @@ -184,15 +209,22 @@ export class CrSearchResultsField extends Component { /** * Mirror the range summary ("1-10 of 23") or the empty-result message into - * a live region that outlives the re-rendered blob, so screen readers hear - * the outcome of a search or a page change. + * the form's live region, so screen readers hear the outcome of a search + * or a page change. The region is cleared first so that an unchanged text + * (a refined search with the same range) is announced again. */ _announceStatus() { const status = this.containerRef.el.querySelector(STATUS_SELECTOR); const text = status ? status.textContent.trim() : ""; - if (this.statusRef.el && this.statusRef.el.textContent !== text) { - this.statusRef.el.textContent = text; + const formEl = this.containerRef.el.closest(".o_form_view"); + const region = formEl && formEl.querySelector(LIVE_REGION_SELECTOR); + if (!region) { + return; } + region.textContent = ""; + requestAnimationFrame(() => { + region.textContent = text; + }); } } diff --git a/spp_change_request_v2/static/src/xml/cr_search_results_field.xml b/spp_change_request_v2/static/src/xml/cr_search_results_field.xml index 3b8f823d9..b4ea3f52c 100644 --- a/spp_change_request_v2/static/src/xml/cr_search_results_field.xml +++ b/spp_change_request_v2/static/src/xml/cr_search_results_field.xml @@ -2,15 +2,12 @@ -
-