-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ui: Release notes signup - Show Alert notification after signup #44856
Changes from 4 commits
b7cf941
6404cd1
c9848e4
b979307
4982a22
fd7139c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,7 @@ export enum AlertLevel { | |
NOTIFICATION, | ||
WARNING, | ||
CRITICAL, | ||
SUCCESS, | ||
} | ||
|
||
export interface AlertInfo { | ||
|
@@ -50,6 +51,9 @@ export interface Alert extends AlertInfo { | |
// ThunkAction which will result in this alert being dismissed. This | ||
// function will be dispatched to the redux store when the alert is dismissed. | ||
dismiss: ThunkAction<Promise<void>, AdminUIState, void>; | ||
// Makes alert to be positioned in the top right corner of the screen instead of | ||
// stretching to full width. | ||
showAsAlert?: boolean; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this be a bit more specific? maybe say |
||
} | ||
|
||
const localSettingsSelector = (state: AdminUIState) => state.localSettings; | ||
|
@@ -240,6 +244,29 @@ export const disconnectedAlertSelector = createSelector( | |
}, | ||
); | ||
|
||
export const emailSubscriptionAlertLocalSetting = new LocalSetting( | ||
"email_subscription_alert", localSettingsSelector, false, | ||
); | ||
|
||
export const emailSubscriptionAlertSelector = createSelector( | ||
emailSubscriptionAlertLocalSetting.selector, | ||
( emailSubscriptionAlert): Alert => { | ||
if (!emailSubscriptionAlert) { | ||
return undefined; | ||
} | ||
return { | ||
level: AlertLevel.SUCCESS, | ||
title: "You successfully signed up for CockroachDB release notes", | ||
text: "You will receive emails about CockroachDB releases and best practices. You can unsubscribe from these emails anytime.", | ||
showAsAlert: true, | ||
dismiss: (dispatch: Dispatch<Action, AdminUIState>) => { | ||
dispatch(emailSubscriptionAlertLocalSetting.set(false)); | ||
return Promise.resolve(); | ||
}, | ||
}; | ||
}, | ||
); | ||
|
||
/** | ||
* Selector which returns an array of all active alerts which should be | ||
* displayed in the alerts panel, which is embedded within the cluster overview | ||
|
@@ -261,6 +288,7 @@ export const panelAlertsSelector = createSelector( | |
*/ | ||
export const bannerAlertsSelector = createSelector( | ||
disconnectedAlertSelector, | ||
emailSubscriptionAlertSelector, | ||
(...alerts: Alert[]): Alert[] => { | ||
return _.without(alerts, null, undefined); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// Copyright 2020 The Cockroach Authors. | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
@require '~src/components/core/index.styl' | ||
|
||
.alert-massage | ||
max-width 470px | ||
font-size 12px | ||
// it is !important because Ant sets margin to 0 | ||
// when closing animation happens and Alert panel | ||
// jumps to the left side. | ||
margin 0 0 0 auto!important | ||
padding-left 48px | ||
border-radius 3px | ||
box-shadow 0 0 10px 0 rgba(71, 88, 114, 0.47) | ||
background-color #ffffff | ||
border-color #ffffff | ||
.alert-massage__icon | ||
font-size 20px | ||
left 16px | ||
.ant-alert-close-icon | ||
margin unset | ||
right 0 | ||
top 0 | ||
padding-right 16px | ||
.ant-alert-message | ||
font-size 14px | ||
color #242a35 | ||
font-weight 600 | ||
|
||
&__close-text | ||
color $colors--neutral-7 | ||
margin $spacing-smaller 0 | ||
font-size $font-size--large | ||
line-height $spacing-smaller | ||
font-weight $font-weight--extra-bold | ||
|
||
.ant-alert-success .alert-massage__icon | ||
color #37a806 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
// Copyright 2020 The Cockroach Authors. | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
import React from "react"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused as to which new components go in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dhartunian , I also thought it has to be in |
||
import { Alert, Icon } from "antd"; | ||
import { Link } from "react-router-dom"; | ||
|
||
import { AlertInfo, AlertLevel } from "src/redux/alerts"; | ||
import "./alertMessage.styl"; | ||
|
||
interface AlertMessageProps extends AlertInfo { | ||
dismiss(): void; | ||
} | ||
|
||
type AlertType = "success" | "info" | "warning" | "error"; | ||
|
||
const mapAlertLevelToType = (alertLevel: AlertLevel): AlertType => { | ||
switch (alertLevel) { | ||
case AlertLevel.SUCCESS: | ||
return "success"; | ||
case AlertLevel.NOTIFICATION: | ||
return "info"; | ||
case AlertLevel.WARNING: | ||
return "warning"; | ||
case AlertLevel.CRITICAL: | ||
return "error"; | ||
default: | ||
return "info"; | ||
} | ||
}; | ||
|
||
const getIconType = (alertLevel: AlertLevel): string => { | ||
switch (alertLevel) { | ||
case AlertLevel.SUCCESS: | ||
return "check-circle"; | ||
case AlertLevel.NOTIFICATION: | ||
return "info-circle"; | ||
case AlertLevel.WARNING: | ||
return "warning"; | ||
case AlertLevel.CRITICAL: | ||
return "close-circle"; | ||
default: | ||
return "info-circle"; | ||
} | ||
}; | ||
|
||
export class AlertMessage extends React.Component<AlertMessageProps> { | ||
static defaultProps: Partial<AlertMessageProps> = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this defaultProps necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, not necessary! Has to be cleaned up. |
||
link: undefined, | ||
text: undefined, | ||
}; | ||
|
||
render() { | ||
const { | ||
level, | ||
dismiss, | ||
link, | ||
title, | ||
text, | ||
} = this.props; | ||
|
||
let description: React.ReactNode = text; | ||
|
||
if (link) { | ||
description = <Link to={link} onClick={dismiss}>{text}</Link>; | ||
} | ||
|
||
const type = mapAlertLevelToType(level); | ||
const iconType = getIconType(level); | ||
return ( | ||
<Alert | ||
className="alert-massage" | ||
message={title} | ||
description={description} | ||
showIcon | ||
icon={<Icon type={iconType} theme="filled" className="alert-massage__icon" />} | ||
closable | ||
onClose={dismiss} | ||
closeText={ | ||
<div className="alert-massage__close-text"> | ||
× | ||
</div> | ||
} | ||
type={type} /> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// Copyright 2020 The Cockroach Authors. | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
export * from "./alertMessage"; |
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.
Do you think this should also be
openState
? It reads a bit weird to dismiss the alert and then check thatdismissedState
isFalse
. What do you think?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.
Hah, good point! Thank you!