Add DevopsAdoptionTableCellFlag component
What does this MR do?
This component is needed in order to display the adoption status of an item, i.e. true or false - within the Devops Adoption feature.
The only way to test this locally would be to test the change in !47164 (merged).
I've opted to open this separate MR in order to iterate while blocked on other parts of the main MR. This also enables me to produce nice and small, easily reviewable MRs
The border is $gray-300
and the fill is $indigo-600
Feature flag rollout issue: #271568 (closed)
Screenshots (strongly suggested)
The component is the small circles within the table
Enabled | Disabled |
---|---|
![]() |
![]() |
Does this MR meet the acceptance criteria?
Conformity
- [-] Changelog entry
- [-] Documentation (if required)
-
Code review guidelines - [-] Merge request performance guidelines
-
Style guides - [-] Database guides
-
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. - [-] Tested in all supported browsers
- [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
- [-] Label as security and @ mention
@gitlab-com/gl-security/appsec
- [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
- [-] Security reports checked/validated by a reviewer from the AppSec team
Related to #271249 (closed)
Merge request reports
Activity
changed milestone to %13.6
added devopsmanage feature flag featureaddition frontend groupoptimize sectiondev labels
added typefeature label
- Resolved by Andrew Fontaine
assigned to @npost
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 5fe2d585 and 55fc408f
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.11 MB 3.11 MB - 0.0 % mainChunk 1.88 MB 1.88 MB - 0.0 %
Note: We do not have exact data for 5fe2d585. So we have used data from: 81a584df.
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
DangerEdited by 🤖 GitLab Bot 🤖- Resolved by Brandon Labuschagne
1 Warning featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on throughput implementation.
- The definition of done documentation.
1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, run the following:
bin/changelog -m 47376 "Add DevopsAdoptionTableCellFlag component"
If you want to create a changelog entry for GitLab EE, run the following instead:
bin/changelog --ee -m 47376 "Add DevopsAdoptionTableCellFlag component"
If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
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 the chosen person is unavailable.
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, mention them as you normally would! Danger does not automatically notify them for you.
Category Reviewer Maintainer frontend Thomas Randolph ( @thomasrandolph
) (UTC-7, 9 hours behind@blabuschagne
)Andrew Fontaine ( @afontaine
) (UTC-5, 7 hours behind@blabuschagne
)If needed, you can retry the
danger-review
job that generated this comment.Generated by
DangerEdited by 🤖 GitLab Bot 🤖@thomasrandolph can you please perform the initial review?
assigned to @thomasrandolph
unassigned @npost
- Resolved by Andrew Fontaine
As a change, I'm happy with the content here!
Everything seems fine to me!I do have one concern, though: this seems generic enough to be re-used anywhere in GitLab where we might want "a little dot that's either filled in or not." Given that it's named specifically for
DevopsAdoption
and the CSS is specific to one page's bundle, there's a non-trivial chance that someone else at the company might also want "empty or filled dots" and wind up re-implementing this component (probably slightly differently, leading to low design cohesion!).So this is the long way around to: Did you consider submitting this as a new Gitlab UI component?
cc: @npost Is this a pattern we might reuse for other "on/off", "yes/no", etc. indicators?
This isn't a blocker by any means, but I'd be disappointed if I were to see this same pattern repeated across GitLab and not centralized into one standard
BooleanFlag
UI component.Approving, but I think this should be seriously considered as a GitLab-pattern component, not just a Admin/DevOps Report pattern.
What is thisBlocker No Concern Level 1-10 1
unassigned @thomasrandolph
@afontaine could you please maintainer review this?
assigned to @afontaine
mentioned in merge request !47491 (merged)
I echo @thomasrandolph's concerns, but it looks like you have a good plan in place @blabuschagne
Looks great
Setting to MWPSenabled an automatic merge when the pipeline for eb694257 succeeds
mentioned in commit 6ecb4aa8
added workflowstaging label
added workflowcanary label and removed workflowstaging label
added workflowproduction label and removed workflowcanary label
added releasedcandidate label
added releasedpublished label and removed releasedcandidate label