updated with code review comments

openshift · Jan 5, 2021 · a6a43b2 · a6a43b2
1 parent 33be128
commit a6a43b2
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 74 deletions.
diff --git a/frontend/packages/console-shared/src/components/dashboard/status-card/AlertItem.tsx b/frontend/packages/console-shared/src/components/dashboard/status-card/AlertItem.tsx
@@ -51,8 +51,8 @@ export const StatusItem: React.FC<StatusItemProps> = ({ Icon, timestamp, message
 
 const AlertItem: React.FC<AlertItemProps> = ({ alert }) => {
   const actionsExtensions = useExtensions<AlertAction>(isAlertAction);
+  const action = getAlertActions(actionsExtensions).get(alert.rule.name);
   const { t } = useTranslation();
-  const action = getAlertActions(actionsExtensions, t).get(alert.rule.name);
   return (
     <StatusItem
       Icon={getSeverityIcon(getAlertSeverity(alert))}

diff --git a/frontend/packages/patternfly/src/components/notification-drawer/notification-entry.tsx b/frontend/packages/patternfly/src/components/notification-drawer/notification-entry.tsx
@@ -1,7 +1,6 @@
 import * as classNames from 'classnames';
 import * as React from 'react';
-import { useTranslation } from 'react-i18next';
-import { TFunction } from 'i18next';
+import i18next from 'i18next';
 import { Link } from 'react-router-dom';
 import {
   BlueArrowCircleUpIcon,
@@ -20,18 +19,18 @@ export enum NotificationTypes {
   update = 'update',
 }
 
-export const notificationTypeString = (type: NotificationTypes, t: TFunction): string => {
+const notificationTypeString = (type: NotificationTypes): string => {
   switch (type) {
     case NotificationTypes.warning:
-      return t('notification-drawer~Warning');
+      return i18next.t('notification-drawer~Warning notification:');
     case NotificationTypes.critical:
-      return t('notification-drawer~Critical');
+      return i18next.t('notification-drawer~Critical notification:');
     case NotificationTypes.success:
-      return t('notification-drawer~Success');
+      return i18next.t('notification-drawer~Success notification:');
     case NotificationTypes.update:
-      return t('notification-drawer~Update');
+      return i18next.t('notification-drawer~Update notification:');
     default:
-      return t('notification-drawer~Info');
+      return i18next.t('notification-drawer~Info notification:');
   }
 };
 
@@ -75,50 +74,43 @@ const NotificationEntry: React.FC<NotificationEntryProps> = ({
   targetPath,
   toggleNotificationDrawer,
   type,
-}) => {
-  const { t } = useTranslation();
-  return (
-    <li
-      className={classNames(`pf-c-notification-drawer__list-item pf-m-hoverable pf-m-${type}`, {
-        'pf-m-read': isRead,
-      })}
-      tabIndex={0}
-      onClick={
-        targetPath
-          ? () => {
-              history.push(targetPath);
-              toggleNotificationDrawer();
-            }
-          : null
-      }
-    >
-      <div className="pf-c-notification-drawer__list-item-header">
-        <span className="pf-c-notification-drawer__list-item-header-icon">
-          <NotificationIcon type={type} />
-        </span>
-        <h4 className="pf-c-notification-drawer__list-item-header-title">
-          <span className="pf-screen-reader">
-            {t('notification-drawer~{{type}} notification:', {
-              type: notificationTypeString(type, t),
-            })}
-          </span>
-          {title}
-        </h4>
-        {actionText && actionPath && (
-          <NotificationAction
-            text={actionText}
-            path={actionPath}
-            onClick={toggleNotificationDrawer}
-          />
-        )}
-      </div>
-      <div className="pf-c-notification-drawer__list-item-description">{description}</div>
-      <div className="pf-c-notification-drawer__list-item-timestamp">
-        {timestamp && <Timestamp simple timestamp={timestamp} />}
-      </div>
-    </li>
-  );
-};
+}) => (
+  <li
+    className={classNames(`pf-c-notification-drawer__list-item pf-m-hoverable pf-m-${type}`, {
+      'pf-m-read': isRead,
+    })}
+    tabIndex={0}
+    onClick={
+      targetPath
+        ? () => {
+            history.push(targetPath);
+            toggleNotificationDrawer();
+          }
+        : null
+    }
+  >
+    <div className="pf-c-notification-drawer__list-item-header">
+      <span className="pf-c-notification-drawer__list-item-header-icon">
+        <NotificationIcon type={type} />
+      </span>
+      <h4 className="pf-c-notification-drawer__list-item-header-title">
+        <span className="pf-screen-reader">{notificationTypeString(type)}</span>
+        {title}
+      </h4>
+      {actionText && actionPath && (
+        <NotificationAction
+          text={actionText}
+          path={actionPath}
+          onClick={toggleNotificationDrawer}
+        />
+      )}
+    </div>
+    <div className="pf-c-notification-drawer__list-item-description">{description}</div>
+    <div className="pf-c-notification-drawer__list-item-timestamp">
+      {timestamp && <Timestamp simple timestamp={timestamp} />}
+    </div>
+  </li>
+);
 
 export type NotificationEntryProps = {
   actionText?: string;

diff --git a/frontend/public/components/notification-drawer.tsx b/frontend/public/components/notification-drawer.tsx
@@ -1,7 +1,7 @@
 import * as _ from 'lodash';
 import * as React from 'react';
 import { useTranslation } from 'react-i18next';
-import { TFunction } from 'i18next';
+import i18next from 'i18next';
 import { connect } from 'react-redux';
 import { Link } from 'react-router-dom';
 import {
@@ -87,9 +87,9 @@ const AlertEmptyState: React.FC<AlertEmptyProps> = ({ drawerToggle }) => {
   );
 };
 
-export const getAlertActions = (actionsExtensions: AlertAction[], t) => {
+export const getAlertActions = (actionsExtensions: AlertAction[]) => {
   const alertActions = new Map().set('AlertmanagerReceiversNotConfigured', {
-    text: t('notification-drawer~Configure'),
+    text: i18next.t('notification-drawer~Configure'),
     path: '/monitoring/alertmanagerconfig',
   });
   actionsExtensions.forEach(({ properties }) =>
@@ -107,13 +107,12 @@ const getAlertNotificationEntries = (
   toggleNotificationDrawer: () => void,
   alertActionExtensions: AlertAction[],
   isCritical: boolean,
-  t: TFunction,
 ): React.ReactNode[] =>
   isLoaded && !_.isEmpty(alertData)
     ? alertData
         .filter((a) => (isCritical ? criticalCompare(a) : otherAlertCompare(a)))
         .map((alert, i) => {
-          const action = getAlertActions(alertActionExtensions, t).get(alert.rule.name);
+          const action = getAlertActions(alertActionExtensions).get(alert.rule.name);
           const alertActionPath = _.isFunction(action?.path) ? action.path(alert) : action?.path;
           return (
             <NotificationEntry
@@ -139,7 +138,6 @@ const getUpdateNotificationEntries = (
   cv: ClusterVersionKind,
   isEditable: boolean,
   toggleNotificationDrawer: () => void,
-  t: TFunction,
 ): React.ReactNode[] => {
   if (!cv || !isEditable) {
     return [];
@@ -155,11 +153,11 @@ const getUpdateNotificationEntries = (
     entries.push(
       <NotificationEntry
         actionPath="/settings/cluster?showVersions"
-        actionText={t('notification-drawer~Update cluster')}
+        actionText={i18next.t('notification-drawer~Update cluster')}
         key="cluster-update"
-        description={updateData[0].version || t('notification-drawer~Unknown')}
+        description={updateData[0].version || i18next.t('notification-drawer~Unknown')}
         type={NotificationTypes.update}
-        title={t('notification-drawer~Cluster update available')}
+        title={i18next.t('notification-drawer~Cluster update available')}
         toggleNotificationDrawer={toggleNotificationDrawer}
         targetPath="/settings/cluster?showVersions"
       />,
@@ -169,14 +167,16 @@ const getUpdateNotificationEntries = (
     entries.push(
       <NotificationEntry
         actionPath="/settings/cluster?showChannels"
-        actionText={t('notification-drawer~Update channel')}
+        actionText={i18next.t('notification-drawer~Update channel')}
         key="channel-update"
-        description={t(
+        description={i18next.t(
           'notification-drawer~The {{newerChannel}} channel is available. If you are interested in updating this cluster to {{newerChannelVersion}} in the future, change the update channel to {{newerChannel}} to receive recommended updates.',
           { newerChannel, newerChannelVersion },
         )}
         type={NotificationTypes.update}
-        title={t('notification-drawer~{{newerChannel}} channel available', { newerChannel })}
+        title={i18next.t('notification-drawer~{{newerChannel}} channel available', {
+          newerChannel,
+        })}
         toggleNotificationDrawer={toggleNotificationDrawer}
         targetPath="/settings/cluster?showChannels"
       />,
@@ -290,23 +290,20 @@ export const ConnectedNotificationDrawer_: React.FC<ConnectedNotificationDrawerP
     clusterVersionData,
     clusterVersionIsEditable,
     toggleNotificationDrawer,
-    t,
   );
   const criticalAlertList: React.ReactNode[] = getAlertNotificationEntries(
     true,
     data,
     toggleNotificationDrawer,
     alertActionExtensions,
     true,
-    t,
   );
   const otherAlertList: React.ReactNode[] = getAlertNotificationEntries(
     loaded,
     data,
     toggleNotificationDrawer,
     alertActionExtensions,
     false,
-    t,
   );
   const [isAlertExpanded, toggleAlertExpanded] = React.useState<boolean>(
     !_.isEmpty(criticalAlertList),

diff --git a/frontend/public/locales/en/notification-drawer.json b/frontend/public/locales/en/notification-drawer.json
@@ -19,10 +19,9 @@
   "{{count}} unread": "{{count}} unread",
   "{{count}} unread_plural": "{{count}} unreads",
   "Close": "Close",
-  "Warning": "Warning",
-  "Critical": "Critical",
-  "Success": "Success",
-  "Update": "Update",
-  "Info": "Info",
-  "{{type}} notification:": "{{type}} notification:"
+  "Warning notification:": "Warning notification:",
+  "Critical notification:": "Critical notification:",
+  "Success notification:": "Success notification:",
+  "Update notification:": "Update notification:",
+  "Info notification:": "Info notification:"
 }