fix(select): Fixes width-issue of select option panel in IE

[macjohnny]

Fixes the select options panel width not matching the select width in IE11

Fixes #11609.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[googlebot]

CLAs look good, thanks!

[crisbeto]

It doesn't look like this solves it completely. The animation still jumps on IE:

[macjohnny]

@crisbeto

It doesn't look like this solves it completely. The animation still jumps on IE:

You are right, however, I think this is a separate issue, although related: #11642

[macjohnny]

@crisbeto

It doesn't look like this solves it completely. The animation still jumps on IE:

The jump in the screen record you show is also present without the fix. If you set the width of the mat-form-field to 400px, the issue is visible. See https://github.com/angular/material2/pull/11801/files#diff-1aafb6bc3df4156eef3db6aeabfeb698R10

Do you want me to further improve the PR to also fix the jumping issue #11642? Or how do you want to proceed?

[crisbeto]

If you know how to fix it, it would be good to fix both of them, otherwise it can be addressed later. Also is there a way to work around having to add an extra wrapper around the select panel?

[macjohnny]

@crisbeto

If you know how to fix it, it would be good to fix both of them, otherwise it can be addressed later.

ok, I will see if I find a solution for that, too.

Also is there a way to work around having to add an extra wrapper around the select panel?

So far, I didn't find a way without adding additional markup (apart from using display: block instead of display: flex on the .mat-overlay-pane). However, I think it is ok to have the .mat-select-panel-wrap with flex-basis: 100%;, since this works the same way as a properly expanding div container, setting the stage for the children.

Or do you see any other solution? Unfortunately, it has alway been a pain using display:flex in IE...

[macjohnny]

concerning #11642:

the reason for the remaining jump of 32px at the end of the animation is the animation transition from min-width: 100% (src/lib/select/select-animations.ts#L43) to min-width: calc(100% + 32px) (src/lib/select/select-animations.ts#L48)

this also happens when starting with an initial state of min-width: calc(100%)

it seems that this is rather a bug in the angular animations itself, since e.g. web-animations-js can handle the transition from min-width: 100% to min-width: calc(100% + 32px) correctly, see this codepen example, which works correctly in IE11.

I filed the following bug report: angular/angular#24545

Here is an example using Angular animations to animate the transition from min-width: 100% to min-width: calc(100% + 32px): https://stackblitz.com/edit/angular-gitter-v3xmvv
It works correctly in Chrome, but it does not work in IE, while animating from e.g. min-width: 100% to min-width: 110% works in both, Chrome and IE.
Nevertheless, animating from min-width: 100% to min-width: calc(110%) also fails in IE.

Using CSS-animation does not work either (it works in chrome, however): https://codepen.io/macjohnny-zh/pen/RJLOWG

In order to fix #11642, I see the following options:

1.) ignore the "little" jump for now, as this PR already reduced the jump in #11642 from "large" to "small", while still keeping the issue open to fix the little jump later
2.) wait for a fix of angular/angular#24545
3.) do not animate from 100% to 100% + 32px but initially start with 100% + 32px, although I personally think this transition looks nice

@crisbeto how do you want to proceed? I suggest to get this PR ready to be merged and fix the little jump of #11642 in a separate PR
I also verified in IE that the test cases I added fail without the fix, and run successfully with the fix.

[crisbeto]

LGTM

[macjohnny]

@crisbeto thanks for reviewing the PR.
I also tried an alternative way of fixing the width-issue by setting flex-direction: column to the .mat-overlay-pane element.
This does not change the correct behavior of Chrome:

However, IE would still get it wrong, now the panel being too large because the max-width: 280px of .mat-select-panel is ignored:

Hence I think the solution of having a .mat-select-panel-wrap with flex-basis: 100%; fixes the problem properly, so this PR should be good to go.

[yogeshgadge]

Those waiting #11609 (comment)

[macjohnny]

@tinayuangao @crisbeto any idea why this PR didn't make it into the 6.3.1 release? is there anything else I need to do?

[crisbeto]

The caretaker just hasn't gotten to it yet.

[macjohnny]

@crisbeto @josephperrott @jelbourn I merged the current master and resolved the conflicts.
Could you please have a look at this PR again? The issue #11609 is still present in the current master. It would be great if this change would make it into 7.0.0

[vivian-hu-zz]

In the effort of trying to run it through the tests of Google internal projects. I found this change made the option panel disappears much faster than before (if you slow the animation, you can see it clearly). It somehow changed the animation behavior and some tests seem very sensitive about the timing.
@macjohnny, can you take a look?

[macjohnny]

@vivian-hu I will try to investigate the animation duration

[macjohnny]

@vivian-hu I found that the panel leave animation was not rendered due to angular/angular#23302
I added a6409a3#diff-06ebba94235af50e354ccf6fce620251R36 to fix this. Now the animation seems to work correctly again.
@crisbeto could you reconsider a review?

[macjohnny]

@vivian-hu could you please have a look at this PR again? it should work correctly and resolve the issue.

[macjohnny]

@crisbeto @vivian-hu @josephperrott I updated the PR with the fix suggested by @striky1 in #11609 (comment), which does not involve markup changes but only the following additional css property:

.mat-select-panel {
  flex: 1 0 auto;
}

So now this PR should fix the issue with minimal changes, while it does not affect the appearance in e.g. Google Chrome. I also tested various widths of the panel and it seems alright, so this PR should be good to go.

I hope this can get merged soon, as this issue has been around for quite a while.

[macjohnny]

@crisbeto @vivian-hu @josephperrott any chance to get this change merged? it is small, fixes the still present IE11 issue with mat-select, is updated with the latest master, and passes all checks.

[crisbeto]

Not sure why the merge-ready label was removed.

[macjohnny]

Not sure why the merge-ready label was removed.

I think because there was an issue with the animation, which was resolved then. But since the change was adapted to a css-only solution, it should be fine now.

[macjohnny]

@crisbeto can you re-approve the now css-only solution?

[macjohnny]

According to #11609 (comment) of @evoltafreak, the css-only solution fails in IE if the options are larger than the trigger width. So I reverted this branch to the markup-solution that (as far as I have tested it) is a working solution.

I saved the css-only solution as a separate branch:
https://github.com/macjohnny/material2/tree/bugfix/11609-select-ie-width-css-only

[macjohnny]

@vivian-hu could you please have a look at this PR again?

[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.}