Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,65 @@ describe('DropdownSelect', function () {
expect(changeSpy).not.toBeCalled();
});
});

describe('focus outline (issue #2428)', () => {
it('should NOT have steal-focus part when a non-first item is initially selected and dropdown is closed', async () => {
const page = await newSpecPage({
components: [DropdownSelect],
html: `
<scale-dropdown-select value="cedric">
<scale-dropdown-select-item value="caspar">Caspar</scale-dropdown-select-item>
<scale-dropdown-select-item value="cedric">Cedric</scale-dropdown-select-item>
<scale-dropdown-select-item value="cem">Cem</scale-dropdown-select-item>
</scale-dropdown-select>`,
});

const selectEl = page.doc.querySelector('scale-dropdown-select');
const baseEl = selectEl.shadowRoot.querySelector(
'[part~="select"]'
) as HTMLElement;

// The part attribute must NOT contain "steal-focus" while dropdown is closed,
// regardless of which item is initially selected.
expect(baseEl.getAttribute('part')).not.toContain('steal-focus');
});

it('should NOT have steal-focus part when the last item is initially selected and dropdown is closed', async () => {
const page = await newSpecPage({
components: [DropdownSelect],
html: `
<scale-dropdown-select value="cem">
<scale-dropdown-select-item value="caspar">Caspar</scale-dropdown-select-item>
<scale-dropdown-select-item value="cedric">Cedric</scale-dropdown-select-item>
<scale-dropdown-select-item value="cem">Cem</scale-dropdown-select-item>
</scale-dropdown-select>`,
});

const selectEl = page.doc.querySelector('scale-dropdown-select');
const baseEl = selectEl.shadowRoot.querySelector(
'[part~="select"]'
) as HTMLElement;

expect(baseEl.getAttribute('part')).not.toContain('steal-focus');
});

it('should NOT have steal-focus part when first item is initially selected and dropdown is closed', async () => {
const page = await newSpecPage({
components: [DropdownSelect],
html: `
<scale-dropdown-select value="caspar">
<scale-dropdown-select-item value="caspar">Caspar</scale-dropdown-select-item>
<scale-dropdown-select-item value="cedric">Cedric</scale-dropdown-select-item>
<scale-dropdown-select-item value="cem">Cem</scale-dropdown-select-item>
</scale-dropdown-select>`,
});

const selectEl = page.doc.querySelector('scale-dropdown-select');
const baseEl = selectEl.shadowRoot.querySelector(
'[part~="select"]'
) as HTMLElement;

expect(baseEl.getAttribute('part')).not.toContain('steal-focus');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,16 @@ export class DropdownSelect {

@Watch('value')
valueChange(newValue) {
this.currentIndex = readOptions(this.hostElement).findIndex(
({ value }) => value === newValue
);
// NOTE: Do NOT set currentIndex here. Setting it while the dropdown is closed
// would add the `steal-focus` CSS part and suppress the focus outline.
// currentIndex is managed by setOpen() and keyboard/mouse handlers.
Comment thread
er-asm marked this conversation as resolved.
Outdated
this.updateInputHidden(newValue);
}

connectedCallback() {
this.currentIndex =
readOptions(this.hostElement).findIndex(
({ value }) => value === this.value
) || -1;
// currentIndex intentionally starts at -1 (the @State() default).
// The dropdown is closed on init, so steal-focus must not be applied.
// currentIndex is set to the selected item's index when the dropdown opens.
}

componentDidRender() {
Expand Down Expand Up @@ -376,6 +375,12 @@ export class DropdownSelect {
this.comboEl.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
this.comboEl.focus();
this.currentIndex = -1;
} else {
// Initialize currentIndex to the currently selected item so that keyboard
// navigation starts from the right position and the item is highlighted.
this.currentIndex = readOptions(this.hostElement).findIndex(
({ value }) => value === this.value
);
}
}

Expand Down
11 changes: 10 additions & 1 deletion packages/visual-tests/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,16 @@ module.exports = async (jestConfig) => {
res.sendFile('index.html');
});

global.__SERVER__ = app.listen(3123);
await new Promise((resolve, reject) => {
const server = app.listen(3123, '0.0.0.0', () => {
console.log(
'\nScale Visual Tests: Storybook server listening on 0.0.0.0:3123'
);
resolve();
});
server.on('error', reject);
global.__SERVER__ = server;
});

await setupPuppeteer(jestConfig);
};
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion packages/visual-tests/src/dropdown-select.visual.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
describe.skip('DropdownSelect', () => {
describe('DropdownSelect', () => {
describe.each(['light', 'dark'])('%p', (mode) => {
beforeAll(async () => {
await global.runColorSetup('components-dropdown-select--standard', mode);
Expand Down
Loading