Migrate GlDropdown to GlDisclosureDropdown
What does this MR do and why?
Migrates one deprecated GlDropdown
as part of the Pajamas Migration Day.
Screenshots or screen recordings
Before | After |
---|---|
![]() |
![]() |
How to set up and validate locally
- Go to any commit's
show
view, like http://gdk.test:3000/flightjs/Flight/-/commit/3c0e45968ab2128668e680edc41be8555246ed5c - Check the n changed files dropdown on the top of the diffs
- Test its search field
- Click an entry and check the view scrolls to that file
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.
Related to #418047 (closed)
Merge request reports
Activity
assigned to @thutterer
- A deleted user
added frontend label
Allure report
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for e5761e65expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 40 | 0 | 6 | 0 | 46 | ✅ | | Data Stores | 22 | 0 | 0 | 0 | 22 | ✅ | | Govern | 48 | 0 | 0 | 0 | 48 | ✅ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | | Plan | 55 | 0 | 0 | 0 | 55 | ✅ | | Verify | 32 | 0 | 0 | 0 | 32 | ✅ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 201 | 0 | 9 | 0 | 210 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for e5761e65expand test summary
+-------------------------------------------------------------+ | suites summary | +--------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +--------+--------+--------+---------+-------+-------+--------+ | Manage | 40 | 0 | 10 | 14 | 50 | ❗ | | Create | 130 | 0 | 36 | 0 | 166 | ✅ | +--------+--------+--------+---------+-------+-------+--------+ | Total | 170 | 0 | 46 | 14 | 216 | ❗ | +--------+--------+--------+---------+-------+-------+--------+
- Resolved by Olena Horal-Koretska
@gitlab-org/manage/foundations/engineering I picked up this migration today and decided to make it a
GlDisclosureDropdown
because this dropdown basically is just a list of links (href
) and clicking one scrolls the page to that#
anchor.I'm wondering now if that was the right choice because I haven't found a documented way to make a
GlDisclosureDropdown
searchable. I had to keep the old "manual" search field with some utility classes. Is this the right approach?Making it a
GlCollapsibleListbox
would be wrong, right?
added FY23Q3 label
- Resolved by Olena Horal-Koretska
@ohoral Are you free to review this MR?
requested review from @ohoral
- Resolved by Olena Horal-Koretska
- Resolved by Olena Horal-Koretska
@thutterer how is it going here? I assume you did not get to it yet - just making sure you're not waiting for anything from my side
Edited by Olena Horal-Koretska
added 3285 commits
-
eb48b235...75ec9185 - 3281 commits from branch
master
- 2cac5b6d - Migrate GlDropdown to GlDisclosureDropdown
- d12a43a3 - Replace JS nav spec with native href check
- 7a78cdcd - Focus first item on keyboard down key
- c0fe01e6 - Add sr-only info for per-file additions and deletions
Toggle commit list-
eb48b235...75ec9185 - 3281 commits from branch
2 Warnings d12a43a3: The commit body should not contain more than 72 characters per line. For more information, take a look at our Commit message guidelines. This merge request changed undocumented Vue components in
vue_shared/
. Please consider creating Stories for these components:app/assets/javascripts/vue_shared/components/diff_stats_dropdown.vue
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 @aalakkad
(UTC+3, 2 hours ahead of author)
@arfedoro
(UTC+1, same timezone as author)
Please check reviewer's status!
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 6fe87ddf and e5761e65
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 4.11 MB 4.12 MB - 0.0 % mainChunk 3.06 MB 3.06 MB - 0.0 % Significant Growth: 3Expand
Entrypoint / Name Size before Size after Diff Diff in percent pages.groups.wikis.diff 196.35 KB 297.43 KB +101.08 KB 51.5 % pages.projects.wikis.diff 199.79 KB 300.87 KB +101.08 KB 50.6 % pages.projects.compare.show 344.57 KB 368.88 KB +24.31 KB 7.1 %
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
,@ohoral
or@pgascouvaillancourt
) for review, if you are unsure about the size increase.Note: We do not have exact data for 6fe87ddf. So we have used data from: 0c3f7034.
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
Danger- Resolved by Olena Horal-Koretska
mentioned in issue gitlab-ui#2374