add system alerts banner component in order to display alerts via jso… #231
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -0,0 +1,95 @@ | |||
| import { default as defaultTheme, default as theme } from '@/components/theme'; | |||
There was a problem hiding this comment.
defaultTheme and theme are referring to the same object default
this can be one import
| @@ -0,0 +1,95 @@ | |||
| import { default as defaultTheme, default as theme } from '@/components/theme'; | |||
| export const SystemAlert: React.FC<Props> = ({ alert, onClose }) => { | ||
| const { backgroundColor, icon, textColor, outline } = AlertVariants[alert.level]; | ||
|
|
||
| const createMarkup = (msg: string) => ({ __html: msg }); |
There was a problem hiding this comment.
this function is pretty pointless. it's used once, not exported, created on every render and all it does is put a string into an object. This can be moved inline to where it's called. something like:
dangerouslySetInnerHTML={{_html:alert.message}}| @@ -0,0 +1,27 @@ | |||
| export type AlertLevel = 'info' | 'warning' | 'critical'; | |||
There was a problem hiding this comment.
please add copyright
| @@ -0,0 +1,31 @@ | |||
| import { ReactElement } from 'react'; | |||
There was a problem hiding this comment.
please add copyright
|
@SaqAsh couple of comments to get started with. I need to double check some of the css theme usage tomorrow |
ciaranschutte
approved these changes
May 21, 2025
SaqAsh
merged commit ceb3d3e
into
overture-stack:feature/bannerComponent
1 check passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Add a flexible SystemAlerts component that supports both env-based and prop-based alert inputs, similarly implemented in virusseq, where inspiration was taken.
Addresses ticket #227
Type of change
Please choose only the relevant options and remove the others.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
You do not need to fullfill all requirements of this checklist, select all that apply:
Additional context
A lot of the code was straight from the virus seq portal, the key difference that I had was that I separated types for ease of readability as well as added an outline which would be one shade darker than the background color for appropriate distinction as per the exemplar.