fix(uiSelect): add class to dropdown instead of simple opacity change… #1936
Conversation
… to remove flicker Previous behaviour and new one with punks are in issue angular-ui#1583.
| visibility: hidden; | ||
| top: 100% !important; | ||
| left: 100% !important; | ||
| z-index: -1 !important; |
There was a problem hiding this comment.
Can you explain why the important is needed?
There was a problem hiding this comment.
Because you set top/left/position with propery of DOM element dropdown[0].style.top dropdown[0].style.left dropdown[0].style.position. z-index is additional it hides drobdown behind 0 z-index. May be an overkill if it's already hidden 3 times but to be sure.
| @@ -411,7 +411,7 @@ uis.directive('uiSelect', | |||
| } | |||
|
|
|||
| // Reset the position of the dropdown. | |||
| dropdown[0].style.opacity = 0; | |||
| dropdown[0].classList.add('ui-select-detached'); | |||
There was a problem hiding this comment.
Is it possible that this class is already being set before it is in this function?
There was a problem hiding this comment.
No it's new class, I haven't added it to templates so it's here first time and we see addition on two sides of if ($select.open) so it's either one or another.
And even if class was added already it will not duplicase as classList.add adds only if not present.
|
What tests do you need specifically? It does not change interaction and just replaces opacity with class toggle? |
|
I can do one test that it will fix by replication issue #1583 itself but I believe to do it for such a small specific issue is a bit of overkill. |
|
I agree on the fact that it looks like a bit of overkill. But as this libary has many issue it is our best check for problems in the future. Could you make it and commit it? |
|
To this same branch, no problem. I need to add comit with |
|
i think |
|
Any updates on this issue? Thanks |
|
Oops, forgot completely, will take some time |
|
We are seeing the same issue as in #1583 however when applying the changes in this PR, it did not help. Looking forward to a proper fix. |
|
Good solution, but doesn't seem to work when $select.$animate is null. |
Jefiozie
dismissed
their stale review
Needs some work after this will proceed review
… to remove flicker
Previous behaviour and new one with punks are in issue #1583.