Use GlCollapsibleListbox in ci_environments_dropdown.vue
Resolves #384285 (closed)
What does this MR do and why?
This MR changes the Vue component used to select environment scope of CI environment variables from GlDropdown
to GlCollapsibleListbox
.
Screenshots or screen recordings
before | after | |
---|---|---|
Closed | ![]() |
![]() |
Open | ![]() |
![]() |
With selection | ![]() |
![]() |
With search query | ![]() |
![]() |
Difference in looks
The dropdown is not occupying the whole width like before. This is because GlCollapsibleList
doesn't (yet?) support specifying additional classes for the toggle button text (it only supports toggleClass
which goes to the button.gl-button-dropdown-toggle
).
Difference in behavior
The dropdown does not clear the search term when it is closed nor when a new environment is created. This is because GlCollapsibleList
does not support programmatically clearing the search input.
before | after |
---|---|
Screen_Recording_2022-12-26_at_4.58.01_PM | Screen_Recording_2022-12-26_at_5.15.04_PM |
How to set up and validate locally
- Go to the CI settings page of a project. For example:
http://localhost:3000/flightjs/Flight/-/settings/ci_cd
- Expand
Variables
section then click onAdd variable
- Validate that you can:
- Select an existing environment
- Search through existing environments
- Create a new environment with a search term that doesn't correspond to any existing environment
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
changed milestone to %15.7
assigned to @eugielimpin
- A deleted user
added frontend label
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer frontend Mireya Andres (
@mgandres
) (UTC+8, same timezone as@eugielimpin
)Olena HK. (
@ohoral
) (UTC+2, 6 hours behind@eugielimpin
)test for spec/features/*
Stanislav Dobrovolschii (
@sdobrovolschii
) (UTC+1, 7 hours behind@eugielimpin
)Maintainer review is optional for test for spec/features/*
UX Katie Macoy (
@katiemacoy
) (UTC+13, 5 hours ahead of@eugielimpin
)Maintainer review is optional for UX To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerBundle size analysis [beta]
This compares changes in bundle size for entry points between the commits e020ee83 and 02f3cb16
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.54 MB 3.54 MB - -0.0 % mainChunk 1.95 MB 1.95 MB - 0.0 % Significant Growth: 1Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.groups.settings.ci_cd.show 1.34 MB 1.45 MB +112.25 KB 8.2 % Significant Reduction: 6Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.admin.application_settings 1.34 MB 1.29 MB -48.52 KB -3.5 % pages.admin.application_settings.advanced_search 1.34 MB 1.29 MB -48.52 KB -3.5 % pages.admin.application_settings.integrations 1.35 MB 1.3 MB -48.52 KB -3.5 % pages.admin.application_settings.metrics_and_profiling 1.34 MB 1.29 MB -48.52 KB -3.5 % pages.admin.application_settings.reporting 1.43 MB 1.38 MB -48.52 KB -3.3 % pages.admin.application_settings.service_usage_data 1.34 MB 1.29 MB -48.52 KB -3.5 %
Your MR has at least one entrypoint growing significantly (more > 1 KB or 2%). If you write new or extend existing features, this is expected and there is nothing to worry about.
Please consider pinging someone from the FE Foundations (
@leipert
,@markrian
,@mikegreiling
,@ohoral
or@pgascouvaillancourt
) for review, if you are unsure about the size increase.Note: We do not have exact data for e020ee83. So we have used data from: 33e342b7.
The intended commit has no webpack pipeline, so we chose the last commit with one before it.Please look at the full report for more details
Read more about how this report works.
Generated by
DangerAllure report
allure-report-publisher
generated test report!e2e-review-qa:
test report for 02f3cb16expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 28 | 0 | 1 | 0 | 29 | ✅ | | Plan | 49 | 0 | 1 | 0 | 50 | ✅ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Manage | 34 | 0 | 3 | 1 | 37 | ❗ | | Framework sanity | 9 | 0 | 1 | 0 | 10 | ✅ | | Govern | 24 | 0 | 5 | 0 | 29 | ✅ | | Verify | 12 | 0 | 1 | 0 | 13 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 156 | 0 | 13 | 1 | 169 | ❗ | +------------------+--------+--------+---------+-------+-------+--------+
added 283 commits
-
fb2fc8a1...d55629d1 - 282 commits from branch
master
- 660f4068 - WIP
-
fb2fc8a1...d55629d1 - 282 commits from branch
added 1 commit
- 384ffffe - Use GlCollapsibleListbox in ci_environments_dropdown
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
added component:dropdown label
added OKR-FY24Q2 label
- Resolved by Justin Ho Tuan Duong
mentioned in work item gitlab-com/www-gitlab-com#14246
changed milestone to %15.8
added missed:15.7 label
- Resolved by Justin Ho Tuan Duong
Hey @bsandlin
Would you mind reviewing this MR? I figured I'd ask you since this feature (if I'm not wrong) is part ofVerify:Pipeline Authoring
requested review from @bsandlin
added UX label
Please wait for Reviewer Roulette to suggest a designer for UX review, and then assign them as Reviewer. This helps evenly distribute reviews across UX.
This message was generated automatically. You're welcome to improve it.
Hi @beckalippert
Could you review this MR for UX, please?I'm asking for a review since the migration does not retain 100% of the behavior and looks of the current implementation. This is mainly because of missing features for
GlCollapsibleListbox
component that we are migrating towards.The differences between the current and new implementations are detailed in the MR description above. I hope it helps with the review
LGTM, but I just want to make sure we want to continue using the term
wildcard
here, @phillipwells :I would expect something like
Create new: zzz
here instead, but maybe "wildcard" is standard terminology? cc @katiemacoyEdited by Becka Lippert@beckalippert I agree, I think
Create new:
is clearer about the action the user is taking here. If it's possible to make this change while we're in here, @eugielimpin, I say go for itcc @katiemacoy
@beckalippert @phillipwells When you're in a merge request and you try to type a label that doesn't exist, here is the behavior:
The word "create" means the same as "new" so you don't need to have both words, usually. (You can't create something that's old, if that makes sense.)
Just wondering how you're using "wildcard" in this case--are you saying they're creating a search string? So if they create "foo" then it will search for
*foo*
?Anyway, this is just food for thought. I'm on vacation and happened to see this MR, so just adding some things to think about. Thanks!
From the doc page for this feature
You can use specific matching to select a particular environment. You can also use wildcard matching () to select a particular environment group, like Review Apps (review/).
So the user can create either a scope that matches a specific environment or a "wildcard" scope that can match multiple environments.
Perhaps "Create wildcard:" is a bit misleading
@beckalippert @eugielimpin @phillipwells @sselhorn - Some expert feedback on why I recommend we keep the
Create wildcard
UI copy here:Create new
is not the same as creating a wildcard in this context, because the user can create either a new environment scope OR a new wildcard for a series of environments.We also use wildcards in context with protecting branches, environments, artefacts, and deploy freezes.
From the docs:
If the environment scope is review/*, then jobs with environment names starting with review/ would have that variable available.
The most specific spec takes precedence over the other wildcard matching. In this case, the review/feature-1 spec takes precedence over review/* and * specs.
Here’s where we worked to improve how environment wildcards are created #5313 (closed) . We also added a link to the docs next to the input label to help users find more info about these wildcards #231529 (closed)
I recommend that a follow-up issue is created to discuss further, since this interaction and language appears in other product areas of CD.
Hope that helps! Let me know
cc @emilybauman@beckalippert Just to be clear, UX is fine with all the visual / UX changes (dropdown is not full width, search does not clear on close / select...) and we don't want to keep any of those?
If that's not the case, we should create a follow-up to at least re-visit these if / when GitLab UI is updated with more functionality. (In its current state, the listbox is still very much WIP and lacks a few features)
I assume the Foundations team is aware of the current listbox component limitations and it will continue to be improved iteratively; can you comment @gitlab-com/gitlab-ux/ux-foundations?
If that's not the case, we should create a follow-up to at least re-visit these if / when GitLab UI is updated with more functionality. (In its current state, the listbox is still very much WIP and lacks a few features)
If you find any issues with the new listbox feel free to create issues for them.
I assume the Foundations team is aware of the current listbox component limitations and it will continue to be improved iteratively
Of course
requested review from @beckalippert
@beckalippert
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.
For more info, please refer to the following links:
added pipeline:mr-approved label
added Technical Writing UI text + 1 deleted label
requested review from @justin_ho and removed review request for @bsandlin
- Resolved by Justin Ho Tuan Duong
- Resolved by Justin Ho Tuan Duong
Added a very small nit that would be good to implement and a comment for UX. Otherwise this LGTM
Back to you for now
removed review request for @justin_ho
added 3635 commits
-
384ffffe...22b9f62f - 3633 commits from branch
master
- 78383efc - Use GlCollapsibleListbox in ci_environments_dropdown
- 02f3cb16 - Revert to using mount
-
384ffffe...22b9f62f - 3633 commits from branch
requested review from @justin_ho
enabled an automatic merge when the pipeline for c00df4bc succeeds
mentioned in commit 5a7ec0fe
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
mentioned in issue #384285 (closed)
added workflowpost-deploy-db-staging label and removed workflowproduction label
added workflowpost-deploy-db-production label and removed workflowpost-deploy-db-staging label
added releasedcandidate label
mentioned in merge request kubitus-project/kubitus-installer!1829 (merged)
added releasedpublished label and removed releasedcandidate label