RC #1557#1558
Merged
Merged
Conversation
…ropdown components Review of: #1554
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Potential fix for pull request finding Review of #1554 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Potential fix for pull request finding Review of #1554 Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reviewer's GuideCombobox, dropdown, and input components are updated to fix flight-search regressions around displayValue rendering and focus behavior, adding synthetic displayValue handling, safer focus routing, a self-clearing programmatic refresh flag, conditional bib autofocus, and more robust displayValue slot detection, with targeted tests and a post-mortem doc. Sequence diagram for synthetic displayValue creation on preselected combobox valuesequenceDiagram
title Synthetic displayValue handling when combobox has value but no optionSelected
actor Consumer
participant AuroCombobox
participant AuroInput
Consumer->>AuroCombobox: set value
AuroCombobox->>AuroCombobox: updated(value)
AuroCombobox->>AuroCombobox: updateTriggerTextDisplay()
AuroCombobox-->>AuroCombobox: [menu.optionSelected is null and this.value is truthy]
AuroCombobox->>AuroInput: append synthetic span slot=displayValue
AuroCombobox->>AuroInput: checkDisplayValueSlotChange()
AuroInput-->>AuroInput: query slot[name="displayValue"] or light DOM
AuroInput-->>AuroInput: set hasDisplayValueContent
AuroInput->>AuroInput: requestUpdate()
AuroInput-->>Consumer: displayValue overlay shows raw value temporarily
Sequence diagram for focus routing on dropdown close and clear button focussequenceDiagram
title Focus management when dropdown closes after option selection
actor User
participant AuroDropdown
participant AuroCombobox
participant AuroInput
User->>AuroDropdown: select option (click)
AuroDropdown->>AuroDropdown: handleDropdownToggle(eventType="click")
AuroDropdown-->>AuroDropdown: isPopoverVisible = false
AuroDropdown-->>AuroDropdown: noFocusRestoreOnClose = true
AuroDropdown-->>AuroDropdown: [skip focusTrigger()]
AuroCombobox->>AuroCombobox: setClearBtnFocus()
AuroCombobox->>AuroInput: focus()
AuroInput-->>AuroInput: delegatesFocus to native input
AuroCombobox->>AuroInput: query .clearBtn in shadowRoot
AuroCombobox->>AuroInput: clearBtn.focus()
AuroInput-->>User: focus ends on clear button instead of document.body
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
checkDisplayValueSlotChange, usingnode.shadowRoot !== nullto detect custom-element content will miss closed-shadow components (whereshadowRootis always null); consider treating anyELEMENT_NODEas content (or using a different heuristic) so icons/closed-shadow components still triggerhasDisplayValueContent = true. - The
_programmaticFilterRefreshself-clear pattern is now duplicated in bothupdated('value')and theavailableOptionslistener; consider extracting a small helper (e.g.setProgrammaticFilterRefreshOnce()) to keep this behavior centralized and reduce the risk of diverging semantics over time.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `checkDisplayValueSlotChange`, using `node.shadowRoot !== null` to detect custom-element content will miss closed-shadow components (where `shadowRoot` is always null); consider treating any `ELEMENT_NODE` as content (or using a different heuristic) so icons/closed-shadow components still trigger `hasDisplayValueContent = true`.
- The `_programmaticFilterRefresh` self-clear pattern is now duplicated in both `updated('value')` and the `availableOptions` listener; consider extracting a small helper (e.g. `setProgrammaticFilterRefreshOnce()`) to keep this behavior centralized and reduce the risk of diverging semantics over time.
## Individual Comments
### Comment 1
<location path="components/combobox/test/auro-combobox.test.js" line_range="3251-3260" />
<code_context>
+ clearBtn.focus = origFocus;
+ });
+
+ // ─── focusin handler only calls this.focus() when composedPath origin is the host ──
+ it('focusin from a shadow-DOM child does not call this.focus()', async () => {
+ const el = await defaultFixture(mobileView);
+ await elementUpdated(el);
+
+ let hostFocusCalled = false;
+ const origFocus = el.focus.bind(el);
+ el.focus = () => { hostFocusCalled = true; origFocus(); };
+
+ // Focus the internal input directly — the composed focusin event
+ // bubbles to the host with composedPath()[0] pointing at the native
+ // input inside auro-input's shadow DOM, not the combobox host.
+ // The handler should NOT call this.focus() in this case.
+ const nativeInput = el.input.shadowRoot.querySelector('input');
+ nativeInput.focus();
+ await elementUpdated(el);
+
+ expect(hostFocusCalled).to.be.false;
+
+ // Cleanup
+ el.focus = origFocus;
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for the positive focusin case where the event originates from the combobox host.
The current test covers the negative case (focusin from a shadow-DOM child). To fully exercise the handler, please also add a test where `composedPath()[0]` is the combobox host (by focusing the host or dispatching a synthetic event) and assert that `el.focus()` is called. This will better protect against regressions if the `event.composedPath()[0] === this` condition is modified.
Suggested implementation:
```javascript
// Cleanup
clearBtn.focus = origFocus;
});
// ─── focusin handler calls this.focus() when composedPath origin is the host ──
it('focusin from the combobox host calls this.focus()', async () => {
const el = await defaultFixture(mobileView);
await elementUpdated(el);
let hostFocusCalled = false;
const origFocus = el.focus.bind(el);
el.focus = () => {
hostFocusCalled = true;
origFocus();
};
// Dispatch a focusin event from the host — the composedPath()[0] should be
// the combobox host, so the handler should call this.focus().
const focusInEvent = new FocusEvent('focusin', {
bubbles: true,
composed: true
});
el.dispatchEvent(focusInEvent);
await elementUpdated(el);
expect(hostFocusCalled).to.be.true;
// Cleanup
el.focus = origFocus;
});
await expect(dropdown.dropdownWidth).to.equal('400px');
});
```
If the test suite is not already running in a browser-like environment that provides `FocusEvent`, you may need to ensure the appropriate polyfill or environment setup is in place. Also, verify that `defaultFixture` and `elementUpdated` are available in this test block and follow the same pattern as the existing negative focusin test to keep the structure consistent.
</issue_to_address>
### Comment 2
<location path="docs/post-mortem/1598671.md" line_range="216" />
<code_context>
+
+## Accompanying Test Changes
+
+Three new tests added to `components/combobox/test/auro-combobox.test.js`:
+
+1. **`programmatic value sync via syncInputValuesAcrossTriggerAndBib does not set _userTyped`** — Verifies that setting `el.value = 'Apples'` programmatically keeps `_userTyped` false (guarded by `_syncingDisplayValue` / `_programmaticFilterRefresh`), preventing the bib from auto-opening on framework bindings.
</code_context>
<issue_to_address>
**issue (typo):** Add an auxiliary verb for grammatical correctness in the tests summary sentence.
The sentence "Three new tests added to `components/combobox/test/auro-combobox.test.js`:" is missing a verb. Please revise to a grammatically complete form, such as "Three new tests were added to ..." or "Three new tests have been added to ...".
```suggestion
Three new tests were added to `components/combobox/test/auro-combobox.test.js`:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jason-capsule42
approved these changes
Jul 16, 2026
|
🎉 This PR is included in version 6.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alaska Airlines Pull Request
Release candidate pull request. See issue #1557 for details.
Review Checklist:
RC Checklist
Testing Checklist:
Browsers
Browsers Support Guide
Dev demo link
Android
iOS
Desktop
Scenarios
Framework playground
**By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.**
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Improve combobox, dropdown, and input displayValue and focus behavior for preset values and framework integrations, and document the flight-search regressions and fixes.
Enhancements:
Documentation:
Tests: