feat(alert): implement alert component
This implements the alert component, described by #454 (closed).
Old description
This implements the alert component, described by #454 (closed), with the exception of the correct variant-specific icons.
Currently, intentionally-ugly placeholder icons are rendered instead, since they can only be correctly implemented once #98 (closed) is resolved. This is tracked in a follow up issue: #491 (closed).
Screenshots
TODO
-
Remove getSvgIconPathContent()
helper, since it's not supposed to be used in components (blocked by #98 (closed)) -
Preserve appearance with and without GitLab CSS ( font-family
notwithstanding) -
Add another story or two (for visual regression testing)
Merge request reports
Activity
changed milestone to %12.5
added Deliverable component:alert devopssecure frontend workflowin dev labels
@pgascouvaillancourt, this is still a WIP (needs tests and some polish, and a better MR description), but would you be able to give this an early, preliminary review, please?
assigned to @pgascouvaillancourt
- Resolved by Paul Gascou-Vaillancourt
added 1 commit
- fb86a07a - Apply suggestion to documentation/components_documentation.js
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Paul Gascou-Vaillancourt
- Resolved by Mark Florian
- Resolved by Mark Florian
- Resolved by Paul Gascou-Vaillancourt
- Resolved by Mark Florian
- Resolved by Paul Gascou-Vaillancourt
- Resolved by Mark Florian
Great work @markrian! This is looking really good!
I left a few minor comments/questions. Back to you!unassigned @pgascouvaillancourt
mentioned in issue gitlab#22992 (closed)
mentioned in merge request gitlab!18369 (merged)
added 64 commits
-
fb86a07a...f702ce98 - 57 commits from branch
master
- 6aa2c092 - feat(alert): Implement alert component
- a0d2d3ea - Don't use new-style buttons
- 7dd9fec4 - Fix eslint errors
- 2bd22990 - Document alert visibility/dismissal usage
- 15cc8083 - Use design-compliant gl-new-button
- 59731f4b - Make dismiss aria-label configurable
- 009befb7 - Add alert component tests
Toggle commit list-
fb86a07a...f702ce98 - 57 commits from branch
- Resolved by Paul Gascou-Vaillancourt
@pgascouvaillancourt, would you give this another review pass, please?
It turns out this is blocked by #98 (closed), but it's now otherwise pretty close to being ready.
I've updated the styles to fight the leakage from the GitLab CSS bundle, but I'm finding the fonts are different. Other components that are marked
followsDesignSystem: true
(e.g., tabs) also appear to suffer from this, but they don't seem to have failed thetest:visual:gitlab
job. Is that expected?Finally, what's the recommended approach for
stories
andexamples
? If I understand correctly, visual regression snapshots are taken fromstories
, whereas theexamples
are used in design.gitlab.com. So both have value, but it seems wasteful to duplicate code between them.
assigned to @pgascouvaillancourt
- Resolved by Paul Gascou-Vaillancourt
unassigned @pgascouvaillancourt
added 25 commits
-
71d2b243...1555d4c6 - 13 commits from branch
master
- d5b23aa2 - feat(alert): Implement alert component
- ee38700c - Don't use new-style buttons
- 29ae8496 - Fix eslint errors
- d5eec811 - Document alert visibility/dismissal usage
- d0cad2a8 - Use design-compliant gl-new-button
- 6e452658 - Make dismiss aria-label configurable
- 7e698a10 - Add alert component tests
- be68ef67 - Preserve styling with GitLab CSS bundle
- c95c3bf3 - Update alert documentation
- fbae0212 - Simplify common class attribute assignment
- b9360a13 - Improve readability of tests
- c9302413 - Fix dismiss icon alignment with GitLab CSS
Toggle commit list-
71d2b243...1555d4c6 - 13 commits from branch
If you'd be so kind to give this another pass, @pgascouvaillancourt!
assigned to @pgascouvaillancourt
mentioned in issue gitlab-org/gitlab-services/design.gitlab.com#423 (closed)
added 23 commits
-
c9302413...bcce2e95 - 9 commits from branch
master
- d77a551a - feat(alert): Implement alert component
- dfa30d44 - Don't use new-style buttons
- b89801be - Fix eslint errors
- 5337651e - Document alert visibility/dismissal usage
- eba1c1d9 - Use design-compliant gl-new-button
- 9b9f27cc - Make dismiss aria-label configurable
- e552562a - Add alert component tests
- 1c98a16e - Preserve styling with GitLab CSS bundle
- 65d9c2ea - Update alert documentation
- 7cc95c97 - Simplify common class attribute assignment
- a7dcf4c9 - Improve readability of tests
- 24f0dff5 - Fix dismiss icon alignment with GitLab CSS
- a94599c3 - Reformat and fix after rebase
- ec6668e9 - Remove superfluous mixin
Toggle commit list-
c9302413...bcce2e95 - 9 commits from branch
added 2 commits
- Resolved by Paul Gascou-Vaillancourt
Great work @markrian, it looks good to me!
I guess that we can't go any further without !644 (merged). Let's keep an eye on this one to see how it progresses.
@andyvolpe perhaps you could do a UX review pass in the meantime? If you do, keep in mind that some icon won't render properly (
success
andtip
if I'm not mistaken).
assigned to @andyvolpe
unassigned @andyvolpe
added 29 commits
-
05885e6a...d97435ea - 12 commits from branch
master
- 0d06ad3d - feat(alert): Implement alert component
- 7171e58f - Don't use new-style buttons
- baec812c - Fix eslint errors
- 15da1729 - Document alert visibility/dismissal usage
- a8b189fc - Use design-compliant gl-new-button
- 6e58e415 - Make dismiss aria-label configurable
- 0878d118 - Add alert component tests
- 66bb25bc - Preserve styling with GitLab CSS bundle
- 6523becf - Update alert documentation
- cf34db80 - Simplify common class attribute assignment
- 887fb08e - Improve readability of tests
- 9e0573e2 - Fix dismiss icon alignment with GitLab CSS
- 8c236e93 - Reformat and fix after rebase
- c3b52bdb - Remove superfluous mixin
- 86d2b8ef - Fix eslint warning
- 4a87b650 - Add two more alert stories
- b0ed049d - Use placeholder SVG icons for now
Toggle commit list-
05885e6a...d97435ea - 12 commits from branch