Skip to content

Commit

Permalink
fix(material-experimental/mdc-select): change max height to show scro…
Browse files Browse the repository at this point in the history
…llability (angular#24129)

Increases the maximum height of the MDC-based select panel in order to partially show the next option so that it is easier to see when the panel is scrollable.

Related to b/211518654.
  • Loading branch information
crisbeto authored Jan 7, 2022
1 parent 3f727a2 commit 557ac0d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/material-experimental/mdc-select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

$mat-select-arrow-size: 5px !default;
$mat-select-arrow-margin: 4px !default;
$mat-select-panel-max-height: 256px !default;
$mat-select-panel-max-height: 275px !default;
$mat-select-placeholder-arrow-space: 2 *
($mat-select-arrow-size + $mat-select-arrow-margin);
$leading-width: 12px !default;
Expand Down
26 changes: 8 additions & 18 deletions src/material-experimental/mdc-select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2237,9 +2237,8 @@ describe('MDC-based MatSelect', () => {
dispatchKeyboardEvent(host, 'keydown', DOWN_ARROW);
}

// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 256 = 520
// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 256 = 520
expect(panel.scrollTop).withContext('Expected scroll to be at the 16th option.').toBe(520);
// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 275 = 501
expect(panel.scrollTop).withContext('Expected scroll to be at the 16th option.').toBe(501);
}));

it('should scroll up to the active option', fakeAsync(() => {
Expand All @@ -2252,7 +2251,6 @@ describe('MDC-based MatSelect', () => {
dispatchKeyboardEvent(host, 'keydown', UP_ARROW);
}

// <top padding> + <option index * height> = 8 + 9 * 48 = 440
// <top padding> + <option index * height> = 8 + 9 * 48 = 440
expect(panel.scrollTop).withContext('Expected scroll to be at the 9th option.').toBe(440);
}));
Expand All @@ -2277,12 +2275,8 @@ describe('MDC-based MatSelect', () => {
// Note that we press down 5 times, but it will skip
// 3 options because the second group is disabled.
// <top padding> + <(option index + group labels) * height> - <panel height> =
// 8 + (9 + 3) * 48 - 256 = 328
// Note that we press down 5 times, but it will skip
// 3 options because the second group is disabled.
// <top padding> + <(option index + group labels) * height> - <panel height> =
// 8 + (9 + 3) * 48 - 256 = 328
expect(panel.scrollTop).withContext('Expected scroll to be at the 9th option.').toBe(328);
// 8 + (9 + 3) * 48 - 275 = 309
expect(panel.scrollTop).withContext('Expected scroll to be at the 9th option.').toBe(309);
}));

it('should scroll to the top when pressing HOME', fakeAsync(() => {
Expand All @@ -2298,7 +2292,6 @@ describe('MDC-based MatSelect', () => {
dispatchKeyboardEvent(host, 'keydown', HOME);
fixture.detectChanges();

// 8px is the top padding of the panel.
// 8px is the top padding of the panel.
expect(panel.scrollTop).withContext('Expected panel to be scrolled to the top').toBe(8);
}));
Expand All @@ -2308,12 +2301,10 @@ describe('MDC-based MatSelect', () => {
fixture.detectChanges();

// <top padding> + <option amount> * <option height> - <panel height> =
// 8 + 30 * 48 - 256 = 1192
// <top padding> + <option amount> * <option height> - <panel height> =
// 8 + 30 * 48 - 256 = 1192
// 8 + 30 * 48 - 275 = 1173
expect(panel.scrollTop)
.withContext('Expected panel to be scrolled to the bottom')
.toBe(1192);
.toBe(1173);
}));

it('should scroll to the active option when typing', fakeAsync(() => {
Expand All @@ -2325,9 +2316,8 @@ describe('MDC-based MatSelect', () => {
}
flush();

// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 256 = 520
// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 256 = 520
expect(panel.scrollTop).withContext('Expected scroll to be at the 16th option.').toBe(520);
// <top padding> + <option index * height> - <panel height> = 8 + 16 * 48 - 275 = 501
expect(panel.scrollTop).withContext('Expected scroll to be at the 16th option.').toBe(501);
}));

it('should scroll to top when going to first option in top group', fakeAsync(() => {
Expand Down

0 comments on commit 557ac0d

Please sign in to comment.