Make search dropdowns consistent

What does this MR do?

Change the dropdowns on search page to new style. This also affects the following pages in the admin area:

• groups
• projects
• users

Screenshots

Before	After

What are the relevant issue numbers?

#28921 (moved), #35424 (closed)

@cperessini I realized here that for one dropdown implementation, the default behavior is to cut off text. I'm guessing that wrapping makes more sense. What do you think?

assigned to @cperessini

mentioned in issue #28921 (moved)

changed the description

@cperessini The group: / user: looks strange to me as well:

Shouldn't this rather look like this?

mentioned in merge request !13626 (merged)

I'm guessing that wrapping makes more sense. What do you think?

@winh Is that happening for the group and project paths? Wrapping is usually better, but I wonder if wrapping may make the paths harder to understand. Would it be easy to try it out?

Shouldn't this rather look like this?

Yes, your proposal is better

Is that happening for the group and project paths?

@cperessini Theoretically yes, but I don't remember a place using that implementation and displaying project paths. The issue dashboard for example uses the Select2 implementation which already wraps project paths.

Would it be easy to try it out?

If you mean, you'd like to try out both versions, you only need to remove white-space: normal here if you you want to have the non-wrapping behavior after checking out this branch.

Shouldn't this rather look like this?

Yes, your proposal is better

I just looked at the JavaScript code and it seems like we get groups and users as one list, so we cannot separate them easily like I suggested. Maybe we should open a follow-up issue for this. What do you think?

@winh the wrapping in the issue dashboard looks good

Maybe we should open a follow-up issue for this. What do you think?

That sounds good

I think your branch already wraps the text, so is it for review then?

I think your branch already wraps the text, so is it for review then?

@cperessini yep

@winh Style looks good

A few questions:

The group and project dropdowns in search results have that behavior where the checkmark only shows up after you change your selection. Is that something to fix in this MR?

A few of the other dropdowns in this MR don't have a checkmark at all. Would that be fixed by using a different dropdown class?

The namespace dropdown doesn't do anything after selecting an option

The group and project dropdowns in search results have that behavior where the checkmark only shows up after you change your selection.

@cperessini I wasn't able to reproduce this. Could you provide me with a URL where that happens?

A few of the other dropdowns in this MR don't have a checkmark at all. Would that be fixed by using a different dropdown class?

Yes, but as silly as it sounds, switching would be a significant effort, so I doubt it is justified here. I would love if we would have for example a "sort order dropdown" component that we could reuse.

The namespace dropdown doesn't do anything after selecting an option

Good find! ...but this bug was introduced more than a year ago (338072cc) and I honestly don't know how to fix it right away. Can we make that a separate issue?

I wasn't able to reproduce this. Could you provide me with a URL where that happens?

It's in the search results page. I actually just realized that the checkmark doesn't even persist when the page refreshes. In the following GIF, when the dropdown disappears the page has been refreshed.

switching would be a significant effort, so I doubt it is justified here. I would love if we would have for example a "sort order dropdown" component that we could reuse.

That makes sense. Let's look into those changes in the future and keep these MRs focused on the style.

Good find! ...but this bug was introduced more than a year ago (338072cc) and I honestly don't know how to fix it right away. Can we make that a separate issue?

Yep! Same as above, let's take care of this in its own issue. I just point this kind of stuff out in case you can easily fix it, but mostly so it gets written down.

@cperessini Thank you for the GIF! This happens only with Any for me which is why I couldn't reproduce earlier. I'll have a look at it. This shouldn't be difficult to fix.

assigned to @winh

added 222 commits

• f3168815...646aae3e - 220 commits from branch master
• 527ee2d8 - Make search dropdowns consistent
• 56fab4a2 - Support null values in GitLabDropdown

Compare with previous version

@cperessini I think I fixed the problem with Any now: any-selection

Do you want to take another look?

assigned to @cperessini

approved this merge request

Everything looks good to me @winh, thanks a lot! Added my approval.

removed assignee

mentioned in merge request !13745 (merged)

assigned to @winh

added 138 commits

• 56fab4a2...1957099b - 135 commits from branch master
• 2585186b - Make search dropdowns consistent
• 5aa43ce2 - Add failing test for selecting item without ID
• 809ca318 - Support null values in GitLabDropdown

Compare with previous version

added 138 commits

• 56fab4a2...1957099b - 135 commits from branch master
• 2585186b - Make search dropdowns consistent
• 5aa43ce2 - Add failing test for selecting item without ID
• 809ca318 - Support null values in GitLabDropdown

Compare with previous version

@annabeldunstone can you please review?

assigned to @annabeldunstone

mentioned in merge request !13595 (merged)

mentioned in issue #37185

@ClemMakesApps do you have time to take this one? I'm a bit behind on deliverables

assigned to @ClemMakesApps

Is this supposed to happen? Whereby the selected namespace changes after you change the sorting dropdown?

Also, is this change only for the admin pages? I couldn't tell from the MR description

assigned to @winh

mentioned in merge request !13590 (merged)

changed the description

Is this supposed to happen? Whereby the selected namespace changes after you change the sorting dropdown?

The namespace dropdown doesn't seem to work at all. I have created https://gitlab.com/gitlab-org/gitlab-ce/issues/37277 since this is happening on master, too.

Also, is this change only for the admin pages? I couldn't tell from the MR description

This also affects the search page. I have added links to the description now.

assigned to @ClemMakesApps

resolved all discussions

added 402 commits

• 809ca318...bf51ab88 - 399 commits from branch master
• c0aeaf1c - Make search dropdowns consistent
• e1b00e38 - Add failing test for selecting item without ID
• f766f8d2 - Support null values in GitLabDropdown

Compare with previous version

added 402 commits

• 809ca318...bf51ab88 - 399 commits from branch master
• c0aeaf1c - Make search dropdowns consistent
• e1b00e38 - Add failing test for selecting item without ID
• f766f8d2 - Support null values in GitLabDropdown

Compare with previous version

LGTM, Thanks @winh!

@winh there are EE conflicts, can you create EE MR?

assigned to @winh

added 217 commits

• 8e479e2d...6fffddab - 214 commits from branch master
• 8b7a70fb - Make search dropdowns consistent
• 3cd1ade4 - Add failing test for selecting item without ID
• 6838fda7 - Support null values in GitLabDropdown

Compare with previous version

changed the description

@ClemMakesApps yes, created now: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2804

I also had to rebase this one because of conflicts. I made sure everything looks still the same afterwards.

assigned to @ClemMakesApps

tests are failing in EE

assigned to @winh

added 417 commits

• 6838fda7...d4dd6b08 - 414 commits from branch master
• 3f6baee7 - Make search dropdowns consistent
• 601a5d73 - Add failing test for selecting item without ID
• a1e992e5 - Support null values in GitLabDropdown

Compare with previous version

I had to rebase because of conflicts. I made sure it still looks the same.

assigned to @ClemMakesApps

LGTM, Thanks @winh!

approved this merge request

merged

mentioned in commit fed7c1ed

mentioned in issue #38077 (closed)