Adding animation for all dropdown

Found some inconsistencies, obviously this is an inconsistency with our classification rather than your proposal, we should see if we can easily include animations for them.

added 1 commit

58fb3097 - removed disabling animations and fixed up tests

Compare with previous version

@lbennett these are known dropdowns. I was working on those. The Repo search dropdown change top value. I think it is already there.

Okay great :) Thanks @nmrony

added 293 commits

58fb3097...491f1375 - 285 commits from branch master
f343b6a5 - adding animation to dropdown when expand
badc95d7 - resolves scss warnings
e857a1a1 - using mixin to define particular keyframe and animation name
6883de38 - replace keyframe animation with property transition
789d0b1f - adding transition in correct places and corrects some media queries
ffbeb261 - disable css animation for test and resolves all rspec fails
97d3c625 - resolves eslint issues
f4091989 - removed disabling animations and fixed up tests

Compare with previous version

unmarked as a Work In Progress

This is a real tough one. Due to the nature of flakey/unstable tests, every time I restart the pipeline, I see some slightly different errors, some of these are unrelated and just due to unstable ci but some of them are also randomly-occuring valid errors due to something not waiting for the dropdown menu to open. I'm going to try and get as many of these by continuously restarting the pipeline until I get a few clear runs, but this will take some real time to become certain that there will not be a lot of failures for other branches when merged.

@lbennett That's why I thought to disable UI animation for test environment. But you are right we also need to test the animation.

Hehe! It certainly would have been easier! Not a problem! Cant probs get this ready for review tomorrow!

added 1 commit

98f61193 - removed disabling animations and fixed up tests

Compare with previous version

added 67 commits

98f61193...1fce08f9 - 59 commits from branch master
2c3b4bb4 - adding animation to dropdown when expand
df329b38 - resolves scss warnings
72e25805 - using mixin to define particular keyframe and animation name
631ea230 - replace keyframe animation with property transition
012afe9f - adding transition in correct places and corrects some media queries
6095aed7 - disable css animation for test and resolves all rspec fails
66c4e9c5 - resolves eslint issues
61b91229 - removed disabling animations and fixed up tests

Compare with previous version

added 58 commits

61b91229...8a6e4152 - 50 commits from branch master
cdd7366e - adding animation to dropdown when expand
ab93200a - resolves scss warnings
9ac100c8 - using mixin to define particular keyframe and animation name
57bb8f64 - replace keyframe animation with property transition
5da4a05e - adding transition in correct places and corrects some media queries
b775ba6c - disable css animation for test and resolves all rspec fails
a151c6c1 - resolves eslint issues
cf32d598 - removed disabling animations and fixed up tests

Compare with previous version

added 60 commits

cf32d598...b60de9c0 - 52 commits from branch master
6bf6f41e - adding animation to dropdown when expand
59784a6b - resolves scss warnings
e39fe405 - using mixin to define particular keyframe and animation name
ad172ac4 - replace keyframe animation with property transition
63f16ac8 - adding transition in correct places and corrects some media queries
1d36c5df - disable css animation for test and resolves all rspec fails
e127c981 - resolves eslint issues
ee7b3323 - removed disabling animations and fixed up tests

Compare with previous version

added 8 commits

4e3017c2 - adding animation to dropdown when expand
6f8576f4 - resolves scss warnings
e829be47 - using mixin to define particular keyframe and animation name
00453924 - replace keyframe animation with property transition
02e13a98 - adding transition in correct places and corrects some media queries
4c9c8d67 - disable css animation for test and resolves all rspec fails
4a91dac8 - resolves eslint issues
f392c98f - removed disabling animations and fixed up tests

Compare with previous version

Seems to be ready to go now.

Frontend: @nmrony and @brycepj

UX: @dimitrieh

assigned to @nmrony

added 1 commit

2c81c643 - replaced margin with transform

Compare with previous version

resolved all discussions

added 368 commits

2c81c643...09567e93 - 359 commits from branch master
c5d9d2aa - adding animation to dropdown when expand
2151cf35 - resolves scss warnings
5b4c9f59 - using mixin to define particular keyframe and animation name
c23f9560 - replace keyframe animation with property transition
18dd7e5d - adding transition in correct places and corrects some media queries
8ce03088 - disable css animation for test and resolves all rspec fails
be85d969 - resolves eslint issues
dc89bcf6 - removed disabling animations and fixed up tests
289dfc2b - replaced margin with transform

Compare with previous version

added 30 commits

289dfc2b...6aec85ac - 21 commits from branch master
d8d6dd4d - adding animation to dropdown when expand
25ef28ab - resolves scss warnings
87a35c9e - using mixin to define particular keyframe and animation name
4d1892c2 - replace keyframe animation with property transition
92d4133d - adding transition in correct places and corrects some media queries
f69a2306 - disable css animation for test and resolves all rspec fails
41079bdb - resolves eslint issues
77517738 - removed disabling animations and fixed up tests
9e04d92e - replaced margin with transform

Compare with previous version

@ClemMakesApps It looks like the dropdown for filter issues are setting display properties directly from JS. This got precedence over transition in CSS. I think we need to restructure the HAML for those dropdown if we want to do it using only CSS

Hmm. Looks like it will require changing droplab. I've created an issue for it in https://gitlab.com/gitlab-org/droplab/issues/1

Is this a direction issue for %8.17?

@ClemMakesApps Nope, no milestone is set yet but hopefully in 9.0. Another thing is we also need to follow a common html structure for dropdown like other dropdown in the site.

added 1146 commits

9e04d92e...57d5a549 - 1137 commits from branch master
5c4520b7 - adding animation to dropdown when expand
0c25f607 - resolves scss warnings
8e67d633 - using mixin to define particular keyframe and animation name
f44b438c - replace keyframe animation with property transition
85ba5998 - adding transition in correct places and corrects some media queries
f9df3671 - disable css animation for test and resolves all rspec fails
8918d49b - resolves eslint issues
f2746140 - removed disabling animations and fixed up tests
de1a298c - replaced margin with transform

Compare with previous version

marked as a Work In Progress from f034f6b3

added 1 commit

07d8aac9 - adds animation to all dropdown except issue filter and new pickaday dropdown

Compare with previous version

added 108 commits

07d8aac9...59f44052 - 98 commits from branch master
a1542f6b - adding animation to dropdown when expand
065e3582 - resolves scss warnings
4eb6a663 - using mixin to define particular keyframe and animation name
717d45b6 - replace keyframe animation with property transition
7ad861ae - adding transition in correct places and corrects some media queries
b60f598d - disable css animation for test and resolves all rspec fails
a9c40379 - resolves eslint issues
25940be1 - removed disabling animations and fixed up tests
fdb430db - replaced margin with transform
1b0ab954 - adds animation to all dropdown except issue filter and new pickaday dropdown

Compare with previous version

@nmrony i wanted to test it out, but i am getting errors loading the gdk. Is that correct? could you check

@dimitrieh Sure. What is the error? I will take a look into it tomorrow.

Update: Could not resist me to check it right away. I found no error. It is running fine on my localhost. BTW, rebased master onto this branch and pushed it.

added 686 commits

1b0ab954...459a97d4 - 676 commits from branch master
1b1741f8 - adding animation to dropdown when expand
6779ab4c - resolves scss warnings
5664b56d - using mixin to define particular keyframe and animation name
0b0ca5c8 - replace keyframe animation with property transition
48e1a23d - adding transition in correct places and corrects some media queries
a5230cd4 - disable css animation for test and resolves all rspec fails
c67220b4 - resolves eslint issues
e1ec0c97 - removed disabling animations and fixed up tests
f1641e58 - replaced margin with transform
3b3ea0d0 - adds animation to all dropdown except issue filter and new pickaday dropdown

Compare with previous version

@nmrony works really nice! could you speed it up though? and decrease the delay between the fade and the move down just a little?

added 1 commit

64536e01 - decreases delay between fadeIn and slideIn

Compare with previous version

added 1 commit

49dd7509 - removed unnecessary css

Compare with previous version

added 1 commit

958f1f60 - slide more for global search dropdown

Compare with previous version

added 1 commit

d34403f0 - decreases delay between fadeIn and slideIn

Compare with previous version

added 236 commits

d34403f0...8cc61d0b - 225 commits from branch master
ac1853a5 - adding animation to dropdown when expand
0347cffd - resolves scss warnings
1717808a - using mixin to define particular keyframe and animation name
59a8cc66 - replace keyframe animation with property transition
bf4973f0 - adding transition in correct places and corrects some media queries
684cc6d5 - disable css animation for test and resolves all rspec fails
8753ce75 - resolves eslint issues
9c0fa922 - removed disabling animations and fixed up tests
c5a19535 - replaced margin with transform
111fcf61 - adds animation to all dropdown except issue filter and new pickaday dropdown
4759717f - decreases delay between fadeIn and slideIn

Compare with previous version

@dimitrieh I decreased the delay and slide it down a little. Please feel free to poke me until it looks perfect to you

added 1 commit

ae25b206 - halved animation timings even more

Compare with previous version

@nmrony This is great, I still played around some more with it myself and halved all animation timings ;) I pushed this commit. Now it feels a bit more snappier, but there are some left overs:

the new search/filter bar doesn't have this animation:

the dropdowns seem to shift up to fast:

perhaps have them only be faded out and not be moved up again when they disappear.

let me know what you think!

@dimitrieh Great you are really awesome Filter dropdown is not animatable at this moment because of a bug and it requires some changing into it. Here it is https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8419#note_22383325 and I have already mention that on https://gitlab.com/gitlab-org/gitlab-ce/issues/26208#note_22872120. Oops looks like I missed to decrease the delay. I think, moving it up is in UX guideline

@nmrony ah, clear!

moving down when summoning is really nice, i just don't think its necessary perse when its fading away

marked the task All builds are passing as completed

marked the task Added for this feature/bug as completed

added 255 commits

ae25b206...90040620 - 243 commits from branch master
d78d449a - adding animation to dropdown when expand
39dd28c9 - resolves scss warnings
63a14f1d - using mixin to define particular keyframe and animation name
51bee2ae - replace keyframe animation with property transition
e7f382f4 - adding transition in correct places and corrects some media queries
e586e7ba - disable css animation for test and resolves all rspec fails
f687ec96 - resolves eslint issues
abddafc7 - removed disabling animations and fixed up tests
fd030769 - replaced margin with transform
bc301f49 - adds animation to all dropdown except issue filter and new pickaday dropdown
c977d909 - decreases delay between fadeIn and slideIn
a72d372a - halved animation timings even more

Compare with previous version

added 2 commits

56d221ba - animate fade only when hide
ed4c9b64 - only fade during hide for global search

Compare with previous version

@nmrony it seems that the dropdowns now fade away instantly, without any animation..

its okey, but wonder if this was your intention... i think a little fade animation is nice..

@nmrony will we get this merged?

@dimitrieh it is fading but the duration so small that it looks like it is disappearing instantly

@nmrony ok, could you try to let the fade out animation be a tad longer.. while not moving the pane anymore when its fading? After which, let's get this merged ?

@kushalpandya in the light of the other css animation issue, will you be able to get this merged?

added frontend ~1434245 labels

changed milestone to %Backlog

removed assignee

added 2376 commits

ed4c9b64...10a77824 - 2361 commits from branch master
9cb07c81 - adding animation to dropdown when expand
d0d8a50a - resolves scss warnings
e6face90 - using mixin to define particular keyframe and animation name
a184901d - replace keyframe animation with property transition
d2996627 - adding transition in correct places and corrects some media queries
f3747cf6 - disable css animation for test and resolves all rspec fails
adfbba05 - resolves eslint issues
49790c07 - removed disabling animations and fixed up tests
3e4f8284 - replaced margin with transform
dcd5874b - adds animation to all dropdown except issue filter and new pickaday dropdown
a79281b5 - decreases delay between fadeIn and slideIn
37f73cd6 - halved animation timings even more
e8609a87 - animate fade only when hide
5ea3a725 - only fade during hide for global search
5f5f2c1b - Increase animation fade-out duration

Compare with previous version

@dimitrieh Can you check-out this branch locally and see if fade-out duration is fine now? I've currently set it to 250ms which appears good to me.

unmarked as a Work In Progress

marked the task Changelog entry added as completed

@kushalpandya did some last minute minor changes ;) let's get this merged

also added changelog

assigned to @kushalpandya

changed milestone to %9.1

marked the task Conform by the merge request performance guides as completed

marked the task Conform by the style guides as completed

marked the task Branch has no merge conflicts with master (if it does - rebase it please) as completed

marked the task Squashed related commits together as completed

added 1 commit

f357c1ad - [ci skip] Add changelog entry

Compare with previous version

assigned to @ClemMakesApps

Master	This MR

Observations

This MR shifts the dropdown of the filtered search up a few pixels (I also realize that it doesn't animate but I read above and I think we can make this change fairly soon after this MR gets merged)
Some dropdowns have a wider gap than before. Not sure if this was intentional or not
Pika (date picker) dropdowns don't seem to animate. Is this intentional?
User dropdown (top right corner) has a larger width on this MR compared to master
Select2 dropdowns will not be animated for this MR

added 1 commit

79596b2c - 1 commit from branch master

Compare with previous version

@kushalpandya can you review my comments/observations above ? Also, can you update the MR description and mention what dropdowns don't animate (Select2, filtered search)? That will really help figuring out what should be working and what is not intended to be changed for this MR.

@dimitrieh What do you think about the offset of the animated dropdowns? Some of them seem fine to me but some seem to have quite a big gap. The discrepancy is not the best

assigned to @kushalpandya

added 1 commit

a745d3ac - 1 commit from branch master

Compare with previous version

added 1 commit

5f23528a - Fix offset for dropdowns

Compare with previous version

@ClemMakesApps I've fixed the offset to be the same as what it was before this MR. Earlier, all dropdowns did from translateY(-5px) to translateY(5px) (where 5px is extra offset which is not intended everywhere), I've changed it to -10px -> 0.

And as you guessed, animation won't work for dropdowns from Filtered Search, Select2 and standard HTML <select> dropdowns, it works everywhere else.

@kushalpandya the user dropdown still seems wider than normal, please ping me when you're ready for another review

added 1 commit

ccf57bb8 - Fix user menu dropdown width

Compare with previous version

@kushalpandya I still wasn't able to get the animation working on the pika dropdown (the date picker when editing an issue)

I also found this weird scrollbar issue that I don't see happening on gitlab.com

@kushalpandya, I noticed that the filtered search dropdowns are a little smaller in width than before.

Please take a look at my comments above. I know you may have inherited some code in this MR but theres a few cases in this MR where different CSS styles are applied that semantically conflict against each other. Let's make sure we get that resolved and then I think this MR will be in a better shape to get merged

added 3 commits

a2dcdf9e - Add dropdown animation timing function
c95f505e - Use timing function variable
67276a45 - Minor fixes after review feedback

Compare with previous version

added 718 commits

67276a45...01adf920 - 697 commits from branch master
354a083b - adding animation to dropdown when expand
bc355649 - resolves scss warnings
7111d0e0 - using mixin to define particular keyframe and animation name
76f713f2 - replace keyframe animation with property transition
7f571578 - adding transition in correct places and corrects some media queries
f712af5a - disable css animation for test and resolves all rspec fails
13489d0e - resolves eslint issues
c40fd1d3 - removed disabling animations and fixed up tests
b9f896f2 - replaced margin with transform
67d72472 - adds animation to all dropdown except issue filter and new pickaday dropdown
a82e2686 - decreases delay between fadeIn and slideIn
77649e3d - halved animation timings even more
e2b13bb6 - animate fade only when hide
39cd79c7 - only fade during hide for global search
7f66712d - Increase animation fade-out duration
410b4d5d - [ci skip] Add changelog entry
c90e6fd8 - Fix offset for dropdowns
089521e8 - Fix user menu dropdown width
ec39d281 - Add dropdown animation timing function
b5168e5c - Use timing function variable
c8d06848 - Minor fixes after review feedback

Compare with previous version

added 3 commits

6c7128de - Initialize datepicker with container config and animation class
54aa80e0 - Add mixins for animation visibility/hide
fa22409a - Move date clear links to right

Compare with previous version

added 3 commits

6c7128de - Initialize datepicker with container config and animation class
54aa80e0 - Add mixins for animation visibility/hide
fa22409a - Move date clear links to right

Compare with previous version

Hi @ClemMakesApps, I've updated this MR with several changes as per your feedback. Also, animating dropdown that is; a) Browser native dropdown b) Is positioned from JS using inline styles would require certain specific changes to work properly. And, search filter, select2 and datepicker dropdowns are of category b).

While I've added animation support for pikaday datepickers using some initialization config and forcing to override inline styles, it would be tricky to do the same for select2 and droplab (and not a boring solution either). Since both rely on lazy loaded data and inline styles due to strict positioning around input. Let me know what you think.

@ClemMakesApps Regarding scrollbar issue that you mentioned in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8419#note_26917998, it is not happening with me.

assigned to @ClemMakesApps

@kushalpandya have you tried using chrome to replicate that issue?

@ClemMakesApps Yes, this was tested in both Chrome and Firefox.

Just pulled the latest code and I don't see that problem anymore. Maybe it was just a fluke on my browser

@kushalpandya filtered search does not seem to be working anymore.

assigned to @kushalpandya

added 157 commits

fa22409a...a62ae94d - 132 commits from branch master
752a84c9 - adding animation to dropdown when expand
8332cbd8 - resolves scss warnings
8e1b0bc2 - using mixin to define particular keyframe and animation name
87f11d47 - replace keyframe animation with property transition
b7ca043b - adding transition in correct places and corrects some media queries
01f59c4f - disable css animation for test and resolves all rspec fails
60a9557d - resolves eslint issues
44432148 - removed disabling animations and fixed up tests
98210ffa - replaced margin with transform
5b34760c - adds animation to all dropdown except issue filter and new pickaday dropdown
e33b92bf - decreases delay between fadeIn and slideIn
be01ffb8 - halved animation timings even more
8265ef3d - animate fade only when hide
08d73d9c - only fade during hide for global search
0114878c - Increase animation fade-out duration
ec14feec - [ci skip] Add changelog entry
2da556f0 - Fix offset for dropdowns
a983fb94 - Fix user menu dropdown width
5601131f - Add dropdown animation timing function
d8a8618e - Use timing function variable
096a0521 - Minor fixes after review feedback
61c41912 - Initialize datepicker with container config and animation class
113064da - Add mixins for animation visibility/hide
5b198fa2 - Move date clear links to right
9eaa1d46 - Fix container class name

Compare with previous version

@ClemMakesApps Ah, didn't realize we changed container class name of search dropdown from .filtered-search-input-container to .filtered-search-box-input-container Updated the MR with fix, pipeline should hopefully pass now.

added 1 commit

8326ae38 - Compensation for dropdown animation duration with 200ms sleep

Compare with previous version

added 1 commit

98cd4903 - Compensation for dropdown animation duration with 200ms sleep

Compare with previous version

changed milestone to %9.2

@kushalpandya the width on the filtered search dropdown seems a little off

master	this MR

added 114 commits

98cd4903...d7a52716 - 87 commits from branch master
cc915cd7 - adding animation to dropdown when expand
eb22d60a - resolves scss warnings
ec139b2d - using mixin to define particular keyframe and animation name
33755987 - replace keyframe animation with property transition
ae8631f6 - adding transition in correct places and corrects some media queries
077699e2 - disable css animation for test and resolves all rspec fails
57ede65e - resolves eslint issues
da350e00 - removed disabling animations and fixed up tests
573dce55 - replaced margin with transform
ab87a5b3 - adds animation to all dropdown except issue filter and new pickaday dropdown
25804067 - decreases delay between fadeIn and slideIn
72a7fe99 - halved animation timings even more
8db28a02 - animate fade only when hide
d623ea1c - only fade during hide for global search
f277ca7b - Increase animation fade-out duration
457ecb0c - [ci skip] Add changelog entry
f33a6763 - Fix offset for dropdowns
e5b743d0 - Fix user menu dropdown width
dafd520c - Add dropdown animation timing function
ed3cde88 - Use timing function variable
7d8a10b9 - Minor fixes after review feedback
e080b967 - Initialize datepicker with container config and animation class
e03914d6 - Add mixins for animation visibility/hide
e96775f1 - Move date clear links to right
ce627f1f - Fix container class name
ff48982f - Compensation for dropdown animation duration with 200ms sleep
c7311e97 - Fix search filter dropdown width

Compare with previous version

added 1 commit

424772a0 - Fix comments toggle dropdown

Compare with previous version

@ClemMakesApps I've fixed width issue along with a few minor changes. Please not that along with above mentioned scenarios, I'm currently not animating comments type dropdown (introduced in 9.1);

It uses Twitter Bootstrap dropdown for toggle, we can animate it by writing custom dropdown toggle, but it would be wise if we do it in another MR, so as of now, this MR is ready for another review.

assigned to @ClemMakesApps

Last few comments @kushalpandya. Thanks for your patience! I think we may want some backend help for removing the sleep

assigned to @kushalpandya

added 327 commits

424772a0...6e1069d5 - 298 commits from branch master
cd68d78d - adding animation to dropdown when expand
2170ff78 - using mixin to define particular keyframe and animation name
cde730b8 - replace keyframe animation with property transition
f3679301 - adding transition in correct places and corrects some media queries
47605127 - disable css animation for test and resolves all rspec fails
7b2aef04 - resolves eslint issues
689a87e2 - removed disabling animations and fixed up tests
92b90f61 - replaced margin with transform
47f5e067 - adds animation to all dropdown except issue filter and new pickaday dropdown
aacee931 - decreases delay between fadeIn and slideIn
db3f0619 - halved animation timings even more
2ba419e5 - animate fade only when hide
fcf98959 - only fade during hide for global search
65d0c43c - Increase animation fade-out duration
d3b6d621 - [ci skip] Add changelog entry
ff31d91c - Fix offset for dropdowns
6a6df56c - Fix user menu dropdown width
b4bc4a0b - Add dropdown animation timing function
84e6c946 - Use timing function variable
a0da5a88 - Minor fixes after review feedback
61e1a785 - Initialize datepicker with container config and animation class
8451147d - Add mixins for animation visibility/hide
3527bf4d - Move date clear links to right
e238abb2 - Fix container class name
36c1c6b6 - Compensation for dropdown animation duration with 200ms sleep
3441fa8c - Fix search filter dropdown width
71c1571b - Fix comments toggle dropdown
10251b74 - Remove unused keyframes
6f786948 - Remove sleep, use find instead of first

Compare with previous version

@ClemMakesApps resolved conflicts, updated with changes that you suggested, left one comment for you. Please check.

assigned to @ClemMakesApps

assigned to @kushalpandya

added 1 commit

ac5d2544 - Refactor style overrides to be more concise

Compare with previous version

resolved all discussions

@ClemMakesApps Sending it your way again, please check.

assigned to @ClemMakesApps

@kushalpandya Can you create another issue to track the remaining dropdowns that don't animate?

I also noticed that we moved the clear start date to the right so that the date picker doesn't block the link. I think that looks good, but I wanted to confirm with UX so we don't end up with an unnecessary regression

Before	After

@ClemMakesApps Sure, I'll do that.

Makes sense to me @ClemMakesApps @kushalpandya - thanks!

Thanks @tauriedavis

This LGTM. I'm going to merge it. @kushalpandya when you've created the issue, please comment below with it

Thanks for your hard work on this!

approved this merge request

merged

mentioned in commit 1f7bc0c5

mentioned in issue #26208 (closed)

@ClemMakesApps I've re-opened the original issue (#26208 (closed)) with updated description to track dropdowns with missing animation.

mentioned in merge request !10887 (merged)

mentioned in issue #33651 (closed)

Adding animation for all dropdown

What does this MR do?

Why was this MR needed?

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Activity

Observations

Admin message

Admin message

Adding animation for all dropdown

What does this MR do?

Why was this MR needed?

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Merge request reports

Activity

Observations