-
Notifications
You must be signed in to change notification settings - Fork 350
refactor(notification): migrate Notification from Flow to TypeScript #4777
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: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import * as React from 'react'; | ||
| import { defineMessages, injectIntl } from 'react-intl'; | ||
| import type { IntlShape } from 'react-intl'; | ||
| import classNames from 'classnames'; | ||
|
|
||
| import { | ||
| AlertCircle, | ||
| InformationCircle, | ||
| CheckmarkCircle, | ||
| AlertTriangle, | ||
| XMark, | ||
| } from '@box/blueprint-web-assets/icons/Medium'; | ||
|
|
||
| import InfoBadge16 from '../../icon/line/InfoBadge16'; | ||
| import CircleCheck16 from '../../icon/line/CircleCheck16'; | ||
| import TriangleAlert16 from '../../icon/line/TriangleAlert16'; | ||
|
|
||
| import XBadge16 from '../../icon/line/XBadge16'; | ||
| import X16 from '../../icon/fill/X16'; | ||
|
|
||
| import type { NotificationType } from '../../common/types/core'; | ||
|
|
||
| import './Notification.scss'; | ||
|
|
||
| // @NOTE: We can't import these constants from ./constant.js because `react-docgen` | ||
| // can't handle imported variables appear in propTypes | ||
| // see https://github.com/reactjs/react-docgen/issues/33 | ||
| const DURATION_SHORT = 'short'; | ||
| const DURATION_LONG = 'long'; | ||
| const OVERFLOW_WRAP = 'wrap'; | ||
| const TYPE_DEFAULT = 'default'; | ||
| const TYPE_INFO = 'info'; | ||
| const TYPE_WARN = 'warn'; | ||
| const TYPE_ERROR = 'error'; | ||
|
|
||
| const DURATION_TIMES = { | ||
| [DURATION_SHORT]: 5000, | ||
| [DURATION_LONG]: 10000, | ||
| }; | ||
|
|
||
| const ICON_RENDERER: Record<NotificationType, (useV2Icons?: boolean) => React.ReactElement> = { | ||
| [TYPE_DEFAULT]: useV2Icons => (useV2Icons ? <InformationCircle /> : <InfoBadge16 />), | ||
| [TYPE_ERROR]: useV2Icons => (useV2Icons ? <AlertCircle /> : <XBadge16 />), | ||
| [TYPE_INFO]: useV2Icons => (useV2Icons ? <CheckmarkCircle /> : <CircleCheck16 />), | ||
| [TYPE_WARN]: useV2Icons => (useV2Icons ? <AlertTriangle /> : <TriangleAlert16 />), | ||
| }; | ||
|
|
||
| const messages = defineMessages({ | ||
| clearNotificationButtonText: { | ||
| defaultMessage: 'Clear Notification', | ||
| description: 'Button to clear notification', | ||
| id: 'boxui.notification.clearNotification', | ||
| }, | ||
| }); | ||
|
|
||
| export interface NotificationProps { | ||
| /** | ||
| * The contents of the `Notification`. | ||
| * - Notification text must be wrapped in a `<span />` tag. | ||
| * - Notification buttons must be the `<Button />` component. | ||
| */ | ||
| children: React.ReactNode; | ||
| /** Additional CSS class for the notification */ | ||
| className?: string; | ||
| /** | ||
| * When set, dictates how long the notification will exist before calling `onClose`. | ||
| * If unset, the notification will not automatically call `onClose`. | ||
| * - `short`: 5s | ||
| * - `long`: 10s | ||
| */ | ||
| duration?: 'short' | 'long'; | ||
| /** Intl object provided by injectIntl */ | ||
| intl: IntlShape; | ||
| /** Function that gets executed when close button is clicked or when duration expires. */ | ||
| onClose?: (event?: React.SyntheticEvent) => void; | ||
| /** | ||
| * Determines notification colors | ||
| * - `default`: black | ||
| * - `info`: green | ||
| * - `warn`: yellow | ||
| * - `error`: red | ||
| */ | ||
| type?: NotificationType; | ||
| /** How notification text overflow is handled */ | ||
| overflow?: 'wrap' | 'ellipsis'; | ||
| /** When true, render Blueprint v2 icons instead of the local icon set */ | ||
| useV2Icons?: boolean; | ||
| } | ||
|
|
||
| class Notification extends React.Component<NotificationProps> { | ||
| static defaultProps: Pick<NotificationProps, 'overflow' | 'type'> = { | ||
| overflow: OVERFLOW_WRAP, | ||
| type: TYPE_DEFAULT, | ||
| }; | ||
|
|
||
| componentDidMount() { | ||
| const { duration, onClose } = this.props; | ||
| this.timeout = duration && onClose ? setTimeout(onClose, DURATION_TIMES[duration]) : null; | ||
| } | ||
|
|
||
| onClose = (event?: React.SyntheticEvent) => { | ||
| const { onClose } = this.props; | ||
| if (this.timeout) { | ||
| clearTimeout(this.timeout); | ||
| } | ||
|
|
||
| if (onClose) { | ||
| onClose(event); | ||
| } | ||
| }; | ||
|
|
||
| getChildren() { | ||
| const { children } = this.props; | ||
| return typeof children === 'string' ? <span>{children}</span> : children; | ||
| } | ||
|
|
||
| timeout: ReturnType<typeof setTimeout> | null; | ||
|
|
||
| render() { | ||
| const contents = this.getChildren(); | ||
| const { intl, type = TYPE_DEFAULT, overflow, className, useV2Icons } = this.props; | ||
| const { formatMessage } = intl; | ||
| const classes = classNames('notification', type, overflow, className); | ||
| const iconRenderer = ICON_RENDERER[type](useV2Icons); | ||
| const iconColor = useV2Icons ? '#222' : '#fff'; | ||
|
|
||
| return ( | ||
| <div className={classes}> | ||
| {React.cloneElement(iconRenderer, { | ||
| color: iconColor, | ||
| height: 20, | ||
| width: 20, | ||
| })} | ||
| {contents} | ||
| <button | ||
| aria-label={formatMessage(messages.clearNotificationButtonText)} | ||
| className="close-btn" | ||
| onClick={this.onClose} | ||
| type="button" | ||
| > | ||
| {useV2Icons ? <XMark height={32} width={32} /> : <X16 />} | ||
| </button> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export default injectIntl(Notification); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import * as React from 'react'; | ||
|
|
||
| import FocusTrap from '../focus-trap'; | ||
| import Portal from '../portal'; | ||
|
|
||
| export interface NotificationsWrapperProps { | ||
| /** Notification elements to render inside the live region */ | ||
| children?: React.ReactNode; | ||
| } | ||
|
|
||
| const NotificationsWrapper = ({ children }: NotificationsWrapperProps) => ( | ||
| // @ts-ignore Portal forwards children and extra HTML attributes at runtime | ||
| <Portal className="notifications-wrapper" aria-live="polite"> | ||
| {children ? <FocusTrap className="notification-container">{children}</FocusTrap> : null} | ||
| </Portal> | ||
| ); | ||
|
|
||
| export default NotificationsWrapper; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,5 @@ | ||||||
| import * as React from 'react'; | ||||||
| import { shallow } from 'enzyme'; | ||||||
|
|
||||||
| import NotificationsWrapper from '../NotificationsWrapper'; | ||||||
| import Notification from '../Notification'; | ||||||
|
|
@@ -8,7 +9,7 @@ describe('components/notification/NotificationsWrapper', () => { | |||||
| const wrapper = shallow(<NotificationsWrapper />); | ||||||
| expect(wrapper.is('Portal')).toBeTruthy(); | ||||||
| expect(wrapper.hasClass('notifications-wrapper')).toBeTruthy(); | ||||||
| expect(wrapper.props('aria-live')).toBeTruthy(); | ||||||
| expect(wrapper.props()).toBeTruthy(); | ||||||
|
Contributor
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. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Restore the
Assert Proposed fix- expect(wrapper.props()).toBeTruthy();
+ expect(wrapper.prop('aria-live')).toBe('polite');📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| }); | ||||||
|
|
||||||
| test('should render a focus trap', () => { | ||||||
|
|
@@ -18,7 +19,7 @@ describe('components/notification/NotificationsWrapper', () => { | |||||
| </NotificationsWrapper>, | ||||||
| ); | ||||||
| const focusTrap = wrapper.find('FocusTrap'); | ||||||
| expect(focusTrap.length).toEqual(1); | ||||||
| expect(focusTrap).toHaveLength(1); | ||||||
| }); | ||||||
|
|
||||||
| test('should not render focusTrap if there are no children', () => { | ||||||
|
|
@@ -34,6 +35,6 @@ describe('components/notification/NotificationsWrapper', () => { | |||||
| </NotificationsWrapper>, | ||||||
| ); | ||||||
|
|
||||||
| expect(wrapper.find('Notification').length).toEqual(2); | ||||||
| expect(wrapper.find('Notification')).toHaveLength(2); | ||||||
| }); | ||||||
| }); | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| // Duration constants | ||
| export const DURATION_SHORT = 'short'; | ||
| export const DURATION_LONG = 'long'; | ||
|
|
||
| // Type constants | ||
| export const TYPE_DEFAULT = 'default'; | ||
| export const TYPE_INFO = 'info'; | ||
| export const TYPE_WARN = 'warn'; | ||
| export const TYPE_ERROR = 'error'; | ||
|
|
||
| // Overflow constants | ||
| export const OVERFLOW_WRAP = 'wrap'; | ||
| export const OVERFLOW_ELLIPSIS = 'ellipsis'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import * as NotificationConstants from './constants'; | ||
| import Notification from './Notification'; | ||
| import NotificationsWrapper from './NotificationsWrapper'; | ||
|
|
||
| export { Notification, NotificationConstants, NotificationsWrapper }; | ||
| export type { NotificationProps } from './Notification'; | ||
| export type { NotificationsWrapperProps } from './NotificationsWrapper'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @flow | ||
| import * as React from 'react'; | ||
| import { IntlProvider } from 'react-intl'; | ||
|
|
||
|
|
||
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: box/box-ui-elements
Length of output: 50375
🏁 Script executed:
Repository: box/box-ui-elements
Length of output: 14300
🌐 Web query:
react-intl 6.6.8 injectIntl TypeScript declaration InjectedIntlProps public component props💡 Result:
In
react-intlversion 6.6.8, the typeInjectedIntlPropsis not available [1][2][3]. It was removed and replaced in earlier major versions (starting from v3) [1][2]. If you are attempting to useinjectIntlwith TypeScript, you should useWrappedComponentPropsinstead ofInjectedIntlProps[1][2]. To correctly type a component wrapped withinjectIntl: 1. Define your component's own props interface (e.g.,MyComponentProps) [2]. 2. Extend your props interface withWrappedComponentProps(which provides theintlprop) [1][2]. 3. Apply this interface to your component [2]. Example usage:typescript import { injectIntl, WrappedComponentProps } from 'react-intl'; interface MyComponentProps { someCustomProp: string; } const MyComponent = ({ someCustomProp, intl }: MyComponentProps & WrappedComponentProps) => { return <div>{intl.formatMessage({ id: 'someId' })} - {someCustomProp}</div>; }; export default injectIntl(MyComponent);For modern React applications usingreact-intl, it is recommended to use theuseIntlhook instead of theinjectIntlhigher-order component, as it provides a more straightforward way to accessintlcontext without requiring complex type wrappers [4].Citations:
Export public props without
intl.NotificationPropsis re-exported, butinjectIntl(Notification)suppliesintl. Separate the internal props type from the exported public props type.🤖 Prompt for AI Agents