Skip to content

fix(NotificationDrawer): add Translation capability #339

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.

Sign up for GitHub

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

Merged
jeff-phillips-18 merged 1 commit into from
May 14, 2018
Merged

fix(NotificationDrawer): add Translation capability #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/core/src/components/Notification/NotificationDrawer/NotificationDrawerEmptyState.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import React from 'react';
import PropTypes from 'prop-types';
import { EmptyState, EmptyStateIcon, EmptyStateTitle } from '../../EmptyState';

const NotificationDrawerEmptyState = ({ title, className, ...props }) => (
<EmptyState>
<EmptyStateIcon name="info" />
<EmptyStateTitle>{title}</EmptyStateTitle>
</EmptyState>
);
NotificationDrawerEmptyState.propTypes = {
/** Additional element css classes */
className: PropTypes.string,
/** Title */
title: PropTypes.string
};
NotificationDrawerEmptyState.defaultProps = {
className: '',
title: 'No Notifications Available'
};

export default NotificationDrawerEmptyState;
2 changes: 2 additions & 0 deletions packages/core/src/components/Notification/NotificationDrawer/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import NotificationDrawerPanelHeading from './NotificationDrawerAccordion/Notifi
import NotificationDrawerPanelTitle from './NotificationDrawerAccordion/NotificationDrawerPanelTitle';
import NotificationDrawerDropDown from './NotificationDrawerAccordion/NotificationDrawerDropDown';
import NotificationDrawerToggle from './NotificationDrawerToggle';
import NotificationDrawerEmptyState from './NotificationDrawerEmptyState';

import NotificationDrawerWrapper from '../Wrappers/NotificationDrawerWrapper';
import NotificationDrawerPanelWrapper from '../Wrappers/NotificationDrawerPanelWrapper';
Expand All @@ -29,6 +30,7 @@ NotificationDrawer.PanelTitle = NotificationDrawerPanelTitle;
NotificationDrawer.Title = NotificationDrawerTitle;
NotificationDrawer.Dropdown = NotificationDrawerDropDown;
NotificationDrawer.Toggle = NotificationDrawerToggle;
NotificationDrawer.EmptyState = NotificationDrawerEmptyState;

export {
NotificationDrawer,
Expand Down
44 changes: 29 additions & 15 deletions packages/core/src/components/Notification/Wrappers/NotificationDrawerPanelWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { NotificationDrawer } from '../NotificationDrawer/index';
import { Icon } from '../../Icon';
import { Button } from '../../Button';
import { MenuItem } from '../../MenuItem';
import { EmptyState, EmptyStateTitle, EmptyStateIcon } from '../../../index';
import { noop } from '../../../common/helpers';
import getIconClass from './Icon.consts';

Expand All @@ -23,14 +22,15 @@ const NotificationDrawerPanelWrapper = ({
onMarkPanelAsRead,
onClickedLink,
onMarkPanelAsClear,
showLoading
showLoading,
translations
}) => {
const unreadCount = notifications.filter(notification => !notification.seen)
.length;

const getUnread = () => {
if (unreadCount !== 1) return `${unreadCount} Unread Events`;
return '1 Unread Event';
if (unreadCount !== 1) return `${unreadCount} ${translations.unreadEvents}`;
return `1 ${translations.unreadEvent}`;
};

const notificationClickHandler = (panel, notification, seen) => {
Expand Down Expand Up @@ -65,7 +65,7 @@ const NotificationDrawerPanelWrapper = ({
id="notification-kebab-hide"
onClick={() => onNotificationHide(panelkey, notification.id)}
>
Hide this notification
{translations.deleteNotification}
</MenuItem>
</NotificationDrawer.Dropdown>
)}
Expand Down Expand Up @@ -100,26 +100,21 @@ const NotificationDrawerPanelWrapper = ({
data-toggle="mark-all-read"
>
<Button bsStyle="link" onClick={() => onMarkPanelAsRead(panelkey)}>
Mark All Read
{translations.readAll}

@cdcabrera cdcabrera May 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gilad215 just curious what happens when someone accidentally replaces the entire translations object instead of merging or Object.assigning it, or is that even a concern? If we are concerned, could a possible solution involve placing checks for the translation object property, or maybe they're, or it is, required?

@glekner glekner May 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translations is not a required prop.
I don't see how someone can accidentally override the default object without actually writing
translations={...}
If someone makes mistakes, thats their responsibility to fix it, because the object's properties are documented in the storybook

@cdcabrera cdcabrera May 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, if a consumer only handles these properties ...

translations: {
  emptyState: 'Lorem Ipsum Available',
  readAll: 'Lorem Ipsum Read',
  clearAll: 'Lorem Ipsum Clear'
}

What's the expected outcome?

@glekner glekner May 10, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title will be empty, Do you think consumers will want to translate only some of the properties? seems weird to me. If they do it by mistake i'm pretty sure they will understand why. Also, having a prop for each component doesn't look like a solution either, this is a very rare case..

</Button>
</NotificationDrawer.PanelActionLink>
)}
<NotificationDrawer.PanelActionLink data-toggle="clear-all">
<Button bsStyle="link" onClick={() => onMarkPanelAsClear(panelkey)}>
<Icon type="pf" name="close" />
Clear All
{translations.clearAll}
</Button>
</NotificationDrawer.PanelActionLink>
</NotificationDrawer.PanelAction>
);

const noNotificationsMessage = (
<NotificationDrawer.PanelBody key="noNotifications">
<EmptyState>
<EmptyStateIcon name="info" />
<EmptyStateTitle>No Notifications Available</EmptyStateTitle>
</EmptyState>
</NotificationDrawer.PanelBody>
<NotificationDrawer.EmptyState title={translations.emptyState} />
);

return (
Expand Down Expand Up @@ -166,7 +161,17 @@ NotificationDrawerPanelWrapper.propTypes = {
/** function() togglePanel Click */
togglePanel: PropTypes.func,
/** show Loading notification Bool */
showLoading: PropTypes.bool
showLoading: PropTypes.bool,
/** translations for Title, EmptyState, Read/Clear */
translations: PropTypes.shape({
title: PropTypes.string,
unreadEvent: PropTypes.string,
unreadEvents: PropTypes.string,
emptyState: PropTypes.string,
readAll: PropTypes.string,
clearAll: PropTypes.string,
deleteNotification: PropTypes.string
})
};
NotificationDrawerPanelWrapper.defaultProps = {
panelkey: '1',
Expand All @@ -181,7 +186,16 @@ NotificationDrawerPanelWrapper.defaultProps = {
onNotificationHide: noop,
onMarkPanelAsClear: noop,
togglePanel: noop,
showLoading: false
showLoading: false,
translations: {
title: 'Notifications',
unreadEvent: 'Unread Event',
unreadEvents: 'Unread Events',
emptyState: 'No Notifications Available',
readAll: 'Mark All Read',
clearAll: 'Clear All',
deleteNotification: 'Hide this notification'
}
};

export default NotificationDrawerPanelWrapper;
30 changes: 22 additions & 8 deletions packages/core/src/components/Notification/Wrappers/NotificationDrawerWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import React from 'react';
import PropTypes from 'prop-types';
import { NotificationDrawer } from '../NotificationDrawer/index';
import { NotificationDrawerPanelWrapper } from './index';
import { EmptyState, EmptyStateIcon, EmptyStateTitle } from '../../EmptyState';
import { noop } from '../../../common/helpers';

const NotificationDrawerWrapper = ({
panels,
translations,
toggleDrawerHide,
toggleDrawerExpand,
isExpandable,
Expand All @@ -20,6 +20,11 @@ const NotificationDrawerWrapper = ({
onMarkPanelAsClear,
onClickedLink
}) => {
const translationsWrapper = {
...NotificationDrawerPanelWrapper.defaultProps.translations,
...translations
};

const notificationPanels = panels.map((panel, i) => (
<NotificationDrawerPanelWrapper
key={i}
Expand All @@ -35,19 +40,17 @@ const NotificationDrawerWrapper = ({
onMarkPanelAsRead={onMarkPanelAsRead}
onMarkPanelAsClear={onMarkPanelAsClear}
showLoading={panel.showLoading}
translations={translationsWrapper}
/>
));
const noNotificationsMessage = (
<EmptyState>
<EmptyStateIcon name="info" />
<EmptyStateTitle>No Notifications Available</EmptyStateTitle>
</EmptyState>
<NotificationDrawer.EmptyState title={translations.emptyState} />
);

return (
<NotificationDrawer expanded={isExpanded}>
<NotificationDrawer.Title
title="Notifications"
title={translations.title}
onCloseClick={() => toggleDrawerHide()}
expandable={isExpandable}
onExpandClick={toggleDrawerExpand}
Expand Down Expand Up @@ -86,7 +89,17 @@ NotificationDrawerWrapper.propTypes = {
/** show Loading notification Bool */
isExpandable: PropTypes.bool,
/** expanded Panel */
expandedPanel: PropTypes.string
expandedPanel: PropTypes.string,
/** translations for Title, EmptyState, Read/Clear */
translations: PropTypes.shape({
title: PropTypes.string,
unreadEvent: PropTypes.string,
unreadEvents: PropTypes.string,
emptyState: PropTypes.string,
readAll: PropTypes.string,
clearAll: PropTypes.string,
deleteNotification: PropTypes.string
})
};

NotificationDrawerWrapper.defaultProps = {
Expand All @@ -102,7 +115,8 @@ NotificationDrawerWrapper.defaultProps = {
onNotificationHide: noop,
onMarkPanelAsClear: noop,
isExpandable: true,
expandedPanel: null
expandedPanel: null,
translations: {}
};

export default NotificationDrawerWrapper;
2 changes: 2 additions & 0 deletions packages/core/src/components/Notification/Wrappers/Wrappers.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
import { Icon } from '../../Icon';
import getIconClass from './Icon.consts';

jest.unmock('./NotificationDrawerPanelWrapper');

const p1Notifications = [
{
id: 12,
Expand Down
52 changes: 22 additions & 30 deletions packages/core/src/components/Notification/Wrappers/__snapshots__/Wrappers.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -891,25 +891,21 @@ exports[`StatefulNotificationDrawerWrapper is working properly 1`] = `
id="1"
>
<div
class="panel-body"
class="blank-slate-pf"
>
<div
class="blank-slate-pf"
class="blank-slate-pf-icon"
>
<div
class="blank-slate-pf-icon"
>
<span
aria-hidden="true"
class="pficon pficon-info"
/>
</div>
<h4
class="h1 blank-slate-pf-title"
>
No Notifications Available
</h4>
<span
aria-hidden="true"
class="pficon pficon-info"
/>
</div>
<h4
class="h1 blank-slate-pf-title"
>
No Notifications Available
</h4>
</div>
</div>
</div>
Expand Down Expand Up @@ -984,25 +980,21 @@ exports[`StatefulToggleNotificationDrawerWrapper is working properly 1`] = `
id="1"
>
<div
class="panel-body"
class="blank-slate-pf"
>
<div
class="blank-slate-pf"
class="blank-slate-pf-icon"
>
<div
class="blank-slate-pf-icon"
>
<span
aria-hidden="true"
class="pficon pficon-info"
/>
</div>
<h4
class="h1 blank-slate-pf-title"
>
No Notifications Available
</h4>
<span
aria-hidden="true"
class="pficon pficon-info"
/>
</div>
<h4
class="h1 blank-slate-pf-title"
>
No Notifications Available
</h4>
</div>
</div>
</div>
Expand Down