fix(select): set select value to trigger height and center text

[andrewseguin]

Issue was that the text div was actually being calculated as 29px, not 30px, because the trigger had a border bottom of 1px. This made the following code incorrect since the option value height was actually 29px:

/**
 * Must adjust for the difference in height between the option and the trigger,
 * so the text will align on the y axis.
 * (SELECT_OPTION_HEIGHT (48) - SELECT_TRIGGER_HEIGHT (30)) / 2 = 9
 */
export const SELECT_OPTION_HEIGHT_ADJUSTMENT = 9;

This change forces the option value to be 30px and centers the text using flex. Tested on Linux Chrome, Linux FF, Windows IE11 & Edge, Mac Chrome, and Mac FF.

Closes #2977

[crisbeto]

Great catch @andrewseguin! I was struggling with reproducing this one.

[crisbeto]

Can you check if long values get ellipsis with this? I had a fix a while ago that removed it, because it didn't seem to work with flexbox.

[andrewseguin]

Just checked, there's an issue with ellipsis. I believe this can be solved by giving the text itself a span of its own that will have the overflow hidden property. I'll add that to the PR shortly

[andrewseguin]

So I added a bit more changes to make it a little more explicit and future-proof. Children of an element with a border end up with different sizes for height: 100% depending on the browser. In our case, value was getting something like 30px on Chrome and Safari and 29px on FF.

I took border-bottom out and just made a new span to handle the look of a border. That way the option element is for sure 30px on all browsers.

I added an inner span to take care of our truncation as well, thanks for the heads up on that.

[crisbeto]

Instead of creating something completely new, couldn't the .mat-select-border share the styles from the .mat-input-underline from input container? Alternatively, it could be pseudo element so we don't have an extra span in the DOM.

[kara]

Nits

[kara]

There's a mixin for these 4 lines in core called mat-fill. Use that?

[andrewseguin]

Almost, I want to have it sink to the bottom of the container so it's missing the top: 0

[kara]

Ha, I can't read. Ignore me!

[kara]

Looks like you moved this line. Still necessary to keep up here?

[andrewseguin]

Missed that one, will remove

[andrewseguin]

I tried getting a mixin to work with both select and input underlines but since they have different structures its hard to get a good common ground. Select needs to position bottom but this messes up on input. If we can find a way to structure them similarly then I think its worth finding a commons style, but perhaps in another PR.

As for using ::after, I felt that it was a little more complex for users to figure out what they need to override to change the stylings (e.g. underline size, color). By having a separate span, it makes it very easy for them to understand.

[kara]

LGTM

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}