-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add session alerts #3919
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
feat: add session alerts #3919
Conversation
|
You can access the deployment of this PR at https://renku-ci-ui-3919.dev.renku.ch |
a5b2f08 to
57fdd41
Compare
leafty
left a comment
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.
Other things to change:
client/src/features/sessionsV2/SessionShowPage/SessionAlerts.tsx
Outdated
Show resolved
Hide resolved
client/src/features/sessionsV2/SessionShowPage/SessionAlerts.tsx
Outdated
Show resolved
Hide resolved
client/src/features/sessionsV2/SessionShowPage/SessionAlerts.tsx
Outdated
Show resolved
Hide resolved
client/src/features/sessionsV2/SessionShowPage/SessionAlerts.tsx
Outdated
Show resolved
Hide resolved
* fix: proper API spec * update code * fix * fix lint
client/src/features/sessionsV2/SessionShowPage/SessionAlerts.tsx
Outdated
Show resolved
Hide resolved
leafty
left a comment
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.
LGTM
leafty
left a comment
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.
This can be fixed in a fix PR:
| if (!alerts || alerts.length === 0) { | ||
| return ( | ||
| <div> | ||
| <Button |
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.
Note: this should not be a button or it should open something that says "no alerts".
|
@wesjdj should we merge this? |
|
Just fyi the backend counterpart has been merged in the data services and the backend components in the data services that are needed to support this will be released in 2.12.0. So the ui part can go out then or in a later release. |
|
Tearing down the temporary RenkuLab deplyoment for this PR. |
/deploy extra-values=enableInternalGitlab=false