-
Notifications
You must be signed in to change notification settings - Fork 58
feat(lib): add Alerts #3348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ouds/main
Are you sure you want to change the base?
feat(lib): add Alerts #3348
Conversation
. . . Handle zoom fix CI
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request implements the Alert component for OUDS Web, introducing two types of alerts (Alert message and Inline alert) with functional and non-functional variants. The PR replaces the old callout styling system with a new alert-based implementation that aligns with the Orange Unified Design System specifications.
Changes:
- Implemented Alert component with
.alert-messageand inline alert variants, supporting 6 status types (negative, positive, info, warning, neutral, accent) - Renamed alert types from "danger"/"success" to "negative"/"positive" throughout the codebase
- Added "expurge" icon to SVG sprites for all brands as the close button icon
- Updated all documentation callouts to use the new Callout component based on alert structure
- Removed old callout SCSS file and deprecated
CalloutDeprecatedDarkVariantscomponent
Reviewed changes
Copilot reviewed 56 out of 74 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scss/_alert.scss | Complete rewrite implementing new alert structure with container classes, icon handling, and status variants |
| site/src/components/shortcodes/Callout.astro | Refactored to use alert-message structure with title support and action links |
| site/src/content/docs/components/alerts.mdx | New comprehensive documentation for both alert types with examples and accessibility guidelines |
| site/src/assets/partials/snippets.js | Updated alert JavaScript examples to use new structure |
| site/static/*/docs/[version]/assets/img/ouds-web-sprite.svg | Added "expurge" icon for close buttons across all brands |
| packages/*/scss/tokens/_composite.scss | Updated $cross-icon-stroke to match expurge icon |
| site/src/scss/_callouts.scss | Removed old callout styles (file deleted) |
| Multiple .mdx files | Updated callout usages from old syntax to new Callout component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class={`bd-callout bd-callout-${type}`}> | ||
| {Content ? <Content /> : <slot />} | ||
| <div class={`alert alert-message alert-${type} ${className ? className : 'my-medium'}`}> | ||
| <div class="alert-icon">{!!title && <p class="visually-hidden">{titleToDisplay}</p>}</div> |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for showing the visually hidden text on line 83 appears incorrect. The condition !!title will only show the visually hidden text when a custom title is provided, but it should also show it when using the default title (titleToDisplay). This means alerts with default titles (Info, Warning, Danger) won't have the visually hidden text for screen readers unless a custom title is explicitly passed. Consider changing the condition to always include the visually hidden text, or use !!titleToDisplay instead.
| <div class="alert-icon">{!!title && <p class="visually-hidden">{titleToDisplay}</p>}</div> | |
| <div class="alert-icon"><p class="visually-hidden">{titleToDisplay}</p></div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title already displays the hidden text, which is not the case when there is a custom title
| ' </div>', | ||
| ' <div class="alert-action-container">', | ||
| ' <button class="btn btn-icon btn-minimal" data-bs-dismiss="alert">', | ||
| ' <svg aria-hidden="true"><use xlink:href="/orange/docs/1.0/assets/img/ouds-web-sprite.svg#expurge"/></svg>', |
Copilot
AI
Feb 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a hardcoded path to the SVG sprite that will not work for all brands. The path "/orange/docs/1.0/assets/img/ouds-web-sprite.svg#expurge" is specific to the Orange brand, but this code should work for all brands (sosh, orange-compact, etc.). Consider using a dynamic path or a configuration-based approach to reference the correct sprite location.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably will be with the Close button that we are implementing.
Types of change
Related issues
Closes #2785.
Context & Motivation
Read #2785.
Description
--bs-close-iconand all references.Questions
Several questions remain:
.fs-bmon bullet list is weird since label-medium is applied on it ?vertical-alignis weird on zoom, maybe to change on reboot side ?Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews