feat(select): align selected option over trigger #2026
Conversation
googlebot
added
the
cla: yes
kara
force-pushed
the
select-offset
branch
2 times, most recently
from
64c57c8
to
503a168
Compare
| import {ConnectedOverlayPositionChange} from '../core/overlay/position/connected-position'; | ||
| import {ConnectedOverlayDirective} from '../core/overlay/overlay-directives'; | ||
| import {ViewportRuler} from '../core/overlay/position/viewport-ruler'; | ||
|
|
There was a problem hiding this comment.
Maybe add a high level comment here explaining that you need to know a handful of style values here in order to properly align the selected item on open.
| private _calculateOverlayPosition(): void { | ||
| this._offsetX = this._isRtl() ? SELECT_PANEL_PADDING_X : -SELECT_PANEL_PADDING_X; | ||
|
|
||
| const panelHeight = this.options.length <= SELECT_MAX_OPTIONS_DISPLAYED |
There was a problem hiding this comment.
Math.min(this.options.length * SELECT_OPTION_HEIGHT, SELECT_PANEL_MAX_HEIGHT)?
| * y-offset so the panel can open fully on-screen. If it still won't fit, | ||
| * sets the offset back to 0 to allow the fallback position to take over. | ||
| */ | ||
| private _checkOverlayWithinViewport(maxScroll: number): void { |
There was a problem hiding this comment.
Why isn't the behavior
"default position" > "fallback position" ( * n) > slide to stay on-screen
With the last one being part of the connected position strategy?
|
|
||
| /** Sets the transform origin point based on the selected option. */ | ||
| private _getOriginBasedOnOption(): string { | ||
| return `50% ${Math.abs(this._offsetY) - 9 + SELECT_OPTION_HEIGHT / 2}px 0px`; |
There was a problem hiding this comment.
Whoops, that shouldn't be hardcoded. It's the heightDiff number. I'll fix!
| select = fixture.debugElement.query(By.css('md-select')).nativeElement; | ||
| }); | ||
|
|
||
| function checkTriggerAlignedWithOption(index: number): void { |
There was a problem hiding this comment.
Add comment
/**
* Asserts that the given option is aligned with the trigger.
* @param index The index of the option.
*/
kara
force-pushed
the
select-offset
branch
2 times, most recently
from
7ade5cd
to
ae9d26c
Compare
|
@jelbourn Comments addressed |
jelbourn
added
pr: lgtm
action: merge
|
LGTM |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
r: @jelbourn