feat(content-explorer-modal-container): add optional info notice #3634
Conversation
|
|
psatala-box
force-pushed
the
add-info-notice-to-content-explorer-modal-container
branch
from
8399665
to
0e54865
Compare
| @@ -552,6 +559,7 @@ class ContentExplorer extends Component { | |||
| }} | |||
| {...contentExplorerProps} | |||
| > | |||
| {isInfoNoticeVisible && <ContentExplorerInfoNotice infoNoticeText={infoNoticeText} />} | |||
There was a problem hiding this comment.
you can use one prop to control this instead of two:
| {isInfoNoticeVisible && <ContentExplorerInfoNotice infoNoticeText={infoNoticeText} />} | |
| {infoNoticeText && <ContentExplorerInfoNotice infoNoticeText={infoNoticeText} />} |
...nt-explorer/content-explorer-modal-container/__tests__/ContentExplorerModalContainer.test.js
Show resolved
Hide resolved
| import InlineNotice from '../../../components/inline-notice'; | ||
|
|
||
| const ContentExplorerInfoNotice = ({ infoNoticeText }) => ( | ||
| <InlineNotice type="info" className="content-explorer-info-notice"> |
There was a problem hiding this comment.
should this use the InlineNotice from blueprint?
There was a problem hiding this comment.
Right, updated. Now the whole implementation is simpler
| import ContentExplorerInfoNotice from '../ContentExplorerInfoNotice'; | ||
|
|
||
| describe('features/content-explorer/content-explorer/ContentExplorerInfoNotice', () => { | ||
| const renderComponent = infoNoticeText => shallow(<ContentExplorerInfoNotice infoNoticeText={infoNoticeText} />); |
There was a problem hiding this comment.
new tests should use react testing library
psatala-box
force-pushed
the
add-info-notice-to-content-explorer-modal-container
branch
from
97668e2
to
5f31508
Compare
| @@ -0,0 +1,25 @@ | |||
| import * as React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import { injectIntl } from 'react-intl'; | |||
There was a problem hiding this comment.
we've been moving towards useIntl now
| <InlineNotice | ||
| variant="info" | ||
| variantIconAriaLabel={intl.formatMessage(messages.infoNoticeIconAriaLabel)} | ||
| className="content-explorer-info-notice" |
There was a problem hiding this comment.
we don't need this class anymore
There was a problem hiding this comment.
we've been writing new files in tsx
although maybe you can write this inline of ContentExplorer since it doesn't need to be a separate component?
There was a problem hiding this comment.
Updated the files to .tsx. I tried inlining it, but came across some issues with injecting i18n in tests, so I decided to simply update to .tsx
...nt-explorer/content-explorer-modal-container/__tests__/ContentExplorerModalContainer.test.js
Show resolved
Hide resolved
psatala-box
force-pushed
the
add-info-notice-to-content-explorer-modal-container
branch
from
ec77f18
to
6b386e8
Compare
psatala-box
deleted the
add-info-notice-to-content-explorer-modal-container
branch
This PR adds an optional informational notice into the Content Explorer Modal Container. By default the notice is not shown. The notice text is passed in
infoNoticeText. The text passed may not be empty for the notice to be shown.