Skip to content

Commit

Permalink
fix: address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
crisbeto committed May 11, 2017
1 parent 6369c1b commit 268a4e4
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/lib/core/option/optgroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ let nextId = 0;
inputs: ['disabled'],
host: {
'class': 'mat-optgroup',
'role': 'group',
'[class.mat-optgroup-disabled]': 'disabled',
'[attr.aria-disabled]': 'disabled.toString()',
'[attr.aria-labelledby]': '_labelId',
Expand Down
5 changes: 5 additions & 0 deletions src/lib/select/select.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,7 @@ describe('MdSelect', () => {
/**
* Asserts that the given option is aligned with the trigger.
* @param index The index of the option.
* @param selectInstance Instance of the `md-select` component to check against.
*/
function checkTriggerAlignedWithOption(index: number, selectInstance =
fixture.componentInstance.select): void {
Expand Down Expand Up @@ -1656,6 +1657,10 @@ describe('MdSelect', () => {
groups = overlayContainerElement.querySelectorAll('md-optgroup') as NodeListOf<HTMLElement>;
});

it('should set the appropriate role', () => {
expect(groups[0].getAttribute('role')).toBe('group');
});

it('should set the `aria-labelledby` attribute', () => {
let group = groups[0];
let label = group.querySelector('label');
Expand Down
28 changes: 14 additions & 14 deletions src/lib/select/select.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ import 'rxjs/add/operator/startWith';
* the trigger element.
*/

/** The fixed height of every option element. */
export const SELECT_ELEMENT_HEIGHT = 48;
/** The fixed height of every option element (option, group header etc.). */
export const SELECT_ITEM_HEIGHT = 48;

/** The max height of the select's overlay panel */
export const SELECT_PANEL_MAX_HEIGHT = 256;

/** The max number of options visible at once in the select panel. */
export const SELECT_MAX_OPTIONS_DISPLAYED =
Math.floor(SELECT_PANEL_MAX_HEIGHT / SELECT_ELEMENT_HEIGHT);
Math.floor(SELECT_PANEL_MAX_HEIGHT / SELECT_ITEM_HEIGHT);

/** The fixed height of the select's trigger element. */
export const SELECT_TRIGGER_HEIGHT = 30;
Expand All @@ -57,7 +57,7 @@ export const SELECT_TRIGGER_HEIGHT = 30;
* Must adjust for the difference in height between the option and the trigger,
* so the text will align on the y axis.
*/
export const SELECT_OPTION_HEIGHT_ADJUSTMENT = (SELECT_ELEMENT_HEIGHT - SELECT_TRIGGER_HEIGHT) / 2;
export const SELECT_OPTION_HEIGHT_ADJUSTMENT = (SELECT_ITEM_HEIGHT - SELECT_TRIGGER_HEIGHT) / 2;

/** The panel's padding on the x-axis */
export const SELECT_PANEL_PADDING_X = 16;
Expand Down Expand Up @@ -732,8 +732,8 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
/** Calculates the scroll position and x- and y-offsets of the overlay panel. */
private _calculateOverlayPosition(): void {
const items = this._getItemCount();
const panelHeight = Math.min(items * SELECT_ELEMENT_HEIGHT, SELECT_PANEL_MAX_HEIGHT);
const scrollContainerHeight = items * SELECT_ELEMENT_HEIGHT;
const panelHeight = Math.min(items * SELECT_ITEM_HEIGHT, SELECT_PANEL_MAX_HEIGHT);
const scrollContainerHeight = items * SELECT_ITEM_HEIGHT;

// The farthest the panel can be scrolled before it hits the bottom
const maxScroll = scrollContainerHeight - panelHeight;
Expand All @@ -753,7 +753,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
// we must only adjust for the height difference between the option element
// and the trigger element, then multiply it by -1 to ensure the panel moves
// in the correct direction up the page.
this._offsetY = (SELECT_ELEMENT_HEIGHT - SELECT_TRIGGER_HEIGHT) / 2 * -1;
this._offsetY = (SELECT_ITEM_HEIGHT - SELECT_TRIGGER_HEIGHT) / 2 * -1;
}

this._checkOverlayWithinViewport(maxScroll);
Expand All @@ -768,8 +768,8 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
*/
_calculateOverlayScroll(selectedIndex: number, scrollBuffer: number,
maxScroll: number): number {
const optionOffsetFromScrollTop = SELECT_ELEMENT_HEIGHT * selectedIndex;
const halfOptionHeight = SELECT_ELEMENT_HEIGHT / 2;
const optionOffsetFromScrollTop = SELECT_ITEM_HEIGHT * selectedIndex;
const halfOptionHeight = SELECT_ITEM_HEIGHT / 2;

// Starts at the optionOffsetFromScrollTop, which scrolls the option to the top of the
// scroll container, then subtracts the scroll buffer to scroll the option down to
Expand Down Expand Up @@ -864,7 +864,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
let optionOffsetFromPanelTop: number;

if (this._scrollTop === 0) {
optionOffsetFromPanelTop = selectedIndex * SELECT_ELEMENT_HEIGHT;
optionOffsetFromPanelTop = selectedIndex * SELECT_ITEM_HEIGHT;
} else if (this._scrollTop === maxScroll) {
const firstDisplayedIndex = this._getItemCount() - SELECT_MAX_OPTIONS_DISPLAYED;
const selectedDisplayIndex = selectedIndex - firstDisplayedIndex;
Expand All @@ -874,12 +874,12 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
// scrolled to the very bottom, this padding is at the top of the panel and
// must be added to the offset.
optionOffsetFromPanelTop =
selectedDisplayIndex * SELECT_ELEMENT_HEIGHT + SELECT_PANEL_PADDING_Y;
selectedDisplayIndex * SELECT_ITEM_HEIGHT + SELECT_PANEL_PADDING_Y;
} else {
// If the option was scrolled to the middle of the panel using a scroll buffer,
// its offset will be the scroll buffer minus the half height that was added to
// center it.
optionOffsetFromPanelTop = scrollBuffer - SELECT_ELEMENT_HEIGHT / 2;
optionOffsetFromPanelTop = scrollBuffer - SELECT_ITEM_HEIGHT / 2;
}

// The final offset is the option's offset from the top, adjusted for the height
Expand All @@ -904,7 +904,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal

const panelHeightTop = Math.abs(this._offsetY);
const totalPanelHeight =
Math.min(this._getItemCount() * SELECT_ELEMENT_HEIGHT, SELECT_PANEL_MAX_HEIGHT);
Math.min(this._getItemCount() * SELECT_ITEM_HEIGHT, SELECT_PANEL_MAX_HEIGHT);
const panelHeightBottom = totalPanelHeight - panelHeightTop - triggerRect.height;

if (panelHeightBottom > bottomSpaceAvailable) {
Expand Down Expand Up @@ -961,7 +961,7 @@ export class MdSelect implements AfterContentInit, OnDestroy, OnInit, ControlVal
/** Sets the transform origin point based on the selected option. */
private _getOriginBasedOnOption(): string {
const originY =
Math.abs(this._offsetY) - SELECT_OPTION_HEIGHT_ADJUSTMENT + SELECT_ELEMENT_HEIGHT / 2;
Math.abs(this._offsetY) - SELECT_OPTION_HEIGHT_ADJUSTMENT + SELECT_ITEM_HEIGHT / 2;
return `50% ${originY}px 0px`;
}

Expand Down

0 comments on commit 268a4e4

Please sign in to comment.