Make dark mode label text and remove label buttons the same color
What does this MR do and why?
Currently in dark mode, non-scoped labels that have a remove button will have different colored label text and remove buttons. This MR makes them both the same color.
CAVEAT: there are still problems in #428092 (closed) with accessible color contrast and scoped labels that are NOT ADDRESSED in this MR.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
![]() |
![]() |
How to set up and validate locally
- start your local gdk
- if your local gdk user is not already using dark mode, go to
http://gdk.test:3000/-/profile/preferences
and turn on dark mode - navigate to an issue with labels. You will observe that their text color and remove button color do not match
- in the terminal, navigate to the gitlab directory in the gdk and check out this branch
- when the changes in this branch are available in the dom, you will see that the labels on the issue have text and remove buttons that are the same color
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 #428092 (closed)
Merge request reports
Activity
added accessibility automation:ux-missing-labels bugux component:label devopsmanage groupfoundations labels pajamasbuild sectiondev typebug + 1 deleted label
mentioned in merge request !134020 (closed)
replaces !134020 (closed)
assigned to @clavimoniere
- A deleted user
added frontend label
1 Warning You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the maintenancepipelines, maintenancerefactor, maintenanceworkflow, documentation, QA labels.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 thomasrandolph
(UTC-6, 2 hours behind
@clavimoniere
)jivanvl
(UTC-6, 2 hours behind
@clavimoniere
)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
Dangerrequested review from @thutterer and @jivanvl
marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 9580a2c7 and 4576b120
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 4.13 MB 4.13 MB - 0.0 % mainChunk 3.08 MB 3.08 MB - 0.0 %
Note: We do not have exact data for 9580a2c7. So we have used data from: 11b108a8.
The target commit was too new, so we used the latest commit from master we have info on.
It might help to rerun thebundle-size-review
job
This might mean that you have a few false positives in this report. If something unrelated to your code changes is reported, you can check this comparison in order to see if they caused this change.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-test-on-gdk:
test report for 4576b120expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 40 | 0 | 6 | 0 | 46 | ✅ | | Data Stores | 18 | 0 | 4 | 0 | 22 | ✅ | | Verify | 8 | 0 | 0 | 0 | 8 | ✅ | | Manage | 13 | 0 | 1 | 0 | 14 | ✅ | | Govern | 35 | 0 | 0 | 0 | 35 | ✅ | | Plan | 55 | 0 | 0 | 0 | 55 | ✅ | | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 173 | 0 | 13 | 0 | 186 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
test report for 4576b120expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Create | 523 | 0 | 115 | 16 | 638 | ❗ | | Plan | 241 | 0 | 10 | 0 | 251 | ✅ | | Release | 15 | 0 | 3 | 0 | 18 | ✅ | | Verify | 150 | 0 | 15 | 3 | 165 | ❗ | | Manage | 152 | 1 | 16 | 5 | 169 | ❌ | | Govern | 187 | 0 | 3 | 3 | 190 | ❗ | | ModelOps | 0 | 0 | 6 | 0 | 6 | ➖ | | Package | 226 | 0 | 15 | 0 | 241 | ✅ | | Systems | 8 | 0 | 0 | 0 | 8 | ✅ | | GitLab Metrics | 2 | 0 | 1 | 1 | 3 | ❗ | | Analytics | 7 | 0 | 0 | 0 | 7 | ✅ | | Fulfillment | 8 | 0 | 69 | 0 | 77 | ✅ | | Monitor | 28 | 0 | 7 | 0 | 35 | ✅ | | Configure | 1 | 0 | 9 | 0 | 10 | ✅ | | Data Stores | 101 | 0 | 15 | 0 | 116 | ✅ | | Growth | 0 | 0 | 6 | 0 | 6 | ➖ | | Secure | 6 | 0 | 3 | 0 | 9 | ✅ | | Framework sanity | 0 | 0 | 3 | 0 | 3 | ➖ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 1655 | 1 | 296 | 28 | 1952 | ❌ | +------------------+--------+--------+---------+-------+-------+--------+
e2e-review-qa:
test report for 4576b120expand test summary
+-----------------------------------------------------------------------+ | suites summary | +------------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +------------------+--------+--------+---------+-------+-------+--------+ | Package | 0 | 0 | 1 | 0 | 1 | ➖ | | Monitor | 4 | 0 | 0 | 0 | 4 | ✅ | | Create | 8 | 0 | 1 | 0 | 9 | ✅ | | Framework sanity | 0 | 0 | 1 | 0 | 1 | ➖ | | Manage | 1 | 0 | 0 | 0 | 1 | ✅ | | Plan | 3 | 0 | 1 | 0 | 4 | ✅ | | Data Stores | 1 | 0 | 1 | 0 | 2 | ✅ | | Govern | 2 | 0 | 0 | 0 | 2 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+ | Total | 19 | 0 | 5 | 0 | 24 | ✅ | +------------------+--------+--------+---------+-------+-------+--------+
mentioned in issue #428092 (closed)
added UX label
added severity4 label
requested review from @seggenberger
- Resolved by Chad Lavimoniere
- Resolved by Chad Lavimoniere
Hey @clavimoniere
This LGTM, I've left you some code simplifications, but from UX approved
@seggenberger
, thanks for approving this merge request.This is the first time the merge request has been 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
removed review request for @seggenberger
@clavimoniere Some end-to-end (E2E) tests should run based on the stage label.
Please start the
trigger-omnibus-and-follow-up-e2e
job in theqa
stage and ensure tests in thefollow-up-e2e:package-and-test-ee
pipeline pass before this MR is merged. (E2E tests are computationally intensive and don't run automatically for every push/rebase, so we ask you to run this job manually at least once.)To run all E2E tests, apply the pipeline:run-all-e2e label and run a new pipeline.
E2E test jobs are allowed to fail due to flakiness. See current failures at the latest pipeline triage issue.
Once done, apply the
emoji on this comment.Team members only: for any questions or help, reach out on the internal
#quality
Slack channel.added pipeline:run-all-e2e label
changed milestone to %16.5
- Resolved by Chad Lavimoniere
frontend looks good to me! Let me know if you need help merging this one, @clavimoniere
removed review request for @jivanvl
removed review request for @thutterer
mentioned in merge request !134027 (merged)
requested review from @pslaughter
closes #428366 (closed)
Looks like @jivanvl has already
'd. I also tested locally and everything looks good to me. Looks like we just need to merge.Starting merge...
enabled an automatic merge when the pipeline for 9a1c2fc0 succeeds
mentioned in commit 58712307
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
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
added releasedpublished label and removed releasedcandidate label
added UX Paper Cuts label
mentioned in merge request kubitus-project/kubitus-installer!2517 (merged)