Skip to content

Commit fbb80fc

Browse files
authored
fix(material/select): scroll to top on last option before option group (#23147)
We have some logic in `mat-autocomplete` that scrolls the list to the top when the user moves to the first option in the first option group. This is slightly better UX, because it shows the group label, rather than stopping just below it. These changes port over the same logic to `mat-select`.
1 parent e134598 commit fbb80fc

File tree

4 files changed

+76
-12
lines changed

4 files changed

+76
-12
lines changed

src/material-experimental/mdc-select/select.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2134,6 +2134,30 @@ describe('MDC-based MatSelect', () => {
21342134
expect(panel.scrollTop).toBe(520, 'Expected scroll to be at the 16th option.');
21352135
}));
21362136

2137+
it('should scroll to top when going to first option in top group', fakeAsync(() => {
2138+
fixture.destroy();
2139+
const groupFixture = TestBed.createComponent(SelectWithGroups);
2140+
groupFixture.detectChanges();
2141+
groupFixture.componentInstance.select.open();
2142+
groupFixture.detectChanges();
2143+
flush();
2144+
2145+
host = groupFixture.debugElement.query(By.css('mat-select'))!.nativeElement;
2146+
panel = overlayContainerElement.querySelector('.mat-mdc-select-panel')! as HTMLElement;
2147+
2148+
for (let i = 0; i < 5; i++) {
2149+
dispatchKeyboardEvent(host, 'keydown', DOWN_ARROW);
2150+
}
2151+
2152+
expect(panel.scrollTop).toBeGreaterThan(0);
2153+
2154+
for (let i = 0; i < 5; i++) {
2155+
dispatchKeyboardEvent(host, 'keydown', UP_ARROW);
2156+
}
2157+
2158+
expect(panel.scrollTop).toBe(0);
2159+
}));
2160+
21372161
});
21382162
});
21392163

src/material-experimental/mdc-select/select.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
MatOption,
2424
MAT_OPTGROUP,
2525
MAT_OPTION_PARENT_COMPONENT,
26+
_countGroupLabelsBeforeOption,
2627
_getOptionScrollPosition,
2728
} from '@angular/material-experimental/mdc-core';
2829
import {CdkOverlayOrigin, ConnectedPosition} from '@angular/cdk/overlay';
@@ -163,14 +164,22 @@ export class MatSelect extends _MatSelectBase<MatSelectChange> implements OnInit
163164

164165
if (option) {
165166
const panel: HTMLElement = this.panel.nativeElement;
167+
const labelCount = _countGroupLabelsBeforeOption(index, this.options, this.optionGroups);
166168
const element = option._getHostElement();
167169

168-
panel.scrollTop = _getOptionScrollPosition(
169-
element.offsetTop,
170-
element.offsetHeight,
171-
panel.scrollTop,
172-
panel.offsetHeight
173-
);
170+
if (index === 0 && labelCount === 1) {
171+
// If we've got one group label before the option and we're at the top option,
172+
// scroll the list to the top. This is better UX than scrolling the list to the
173+
// top of the option, because it allows the user to read the top group's label.
174+
panel.scrollTop = 0;
175+
} else {
176+
panel.scrollTop = _getOptionScrollPosition(
177+
element.offsetTop,
178+
element.offsetHeight,
179+
panel.scrollTop,
180+
panel.offsetHeight
181+
);
182+
}
174183
}
175184
}
176185

src/material/select/select.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2177,6 +2177,30 @@ describe('MatSelect', () => {
21772177
expect(panel.scrollTop).toBe(512, 'Expected scroll to be at the 16th option.');
21782178
}));
21792179

2180+
it('should scroll to top when going to first option in top group', fakeAsync(() => {
2181+
fixture.destroy();
2182+
const groupFixture = TestBed.createComponent(SelectWithGroups);
2183+
groupFixture.detectChanges();
2184+
groupFixture.componentInstance.select.open();
2185+
groupFixture.detectChanges();
2186+
flush();
2187+
2188+
host = groupFixture.debugElement.query(By.css('mat-select'))!.nativeElement;
2189+
panel = overlayContainerElement.querySelector('.mat-select-panel')! as HTMLElement;
2190+
2191+
for (let i = 0; i < 5; i++) {
2192+
dispatchKeyboardEvent(host, 'keydown', DOWN_ARROW);
2193+
}
2194+
2195+
expect(panel.scrollTop).toBeGreaterThan(0);
2196+
2197+
for (let i = 0; i < 5; i++) {
2198+
dispatchKeyboardEvent(host, 'keydown', UP_ARROW);
2199+
}
2200+
2201+
expect(panel.scrollTop).toBe(0);
2202+
}));
2203+
21802204
});
21812205
});
21822206

src/material/select/select.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1204,12 +1204,19 @@ export class MatSelect extends _MatSelectBase<MatSelectChange> implements OnInit
12041204
const labelCount = _countGroupLabelsBeforeOption(index, this.options, this.optionGroups);
12051205
const itemHeight = this._getItemHeight();
12061206

1207-
this.panel.nativeElement.scrollTop = _getOptionScrollPosition(
1208-
(index + labelCount) * itemHeight,
1209-
itemHeight,
1210-
this.panel.nativeElement.scrollTop,
1211-
SELECT_PANEL_MAX_HEIGHT
1212-
);
1207+
if (index === 0 && labelCount === 1) {
1208+
// If we've got one group label before the option and we're at the top option,
1209+
// scroll the list to the top. This is better UX than scrolling the list to the
1210+
// top of the option, because it allows the user to read the top group's label.
1211+
this.panel.nativeElement.scrollTop = 0;
1212+
} else {
1213+
this.panel.nativeElement.scrollTop = _getOptionScrollPosition(
1214+
(index + labelCount) * itemHeight,
1215+
itemHeight,
1216+
this.panel.nativeElement.scrollTop,
1217+
SELECT_PANEL_MAX_HEIGHT
1218+
);
1219+
}
12131220
}
12141221

12151222
protected _positioningSettled() {

0 commit comments

Comments
 (0)