Skip to content

Commit

Permalink
Add improvements suggested during code review
Browse files Browse the repository at this point in the history
  • Loading branch information
machadoum committed Dec 14, 2022
1 parent 98ae5ee commit e6c6a8d
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ export const CellActions: React.FC<CellActionsProps> = ({
'No CellActionsContext found. Please wrap the application with CellActionsContextProvider'
);
return Promise.resolve([]);
} else {
return context
.getCompatibleActions(triggerId, actionContext)
.then((actions) => orderBy(['order', 'id'], ['asc', 'asc'], actions));
}

return context
.getCompatibleActions(triggerId, actionContext)
.then((actions) => orderBy(['order', 'id'], ['asc', 'asc'], actions));
}, [context, triggerId, actionContext]);

if (mode === CellActionsMode.HOVER_POPOVER) {
Expand Down Expand Up @@ -131,8 +131,6 @@ export const CellActions: React.FC<CellActionsProps> = ({
<div ref={extraContentNodeRef} />
</div>
);
} else {
return <>Not implemented</>;
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@

import { EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import React from 'react';
import { i18n } from '@kbn/i18n';
import { SHOW_MORE_ACTIONS } from './translations';

interface ExtraActionsButtonProps {
onClick: () => void;
showTooltip: boolean;
}

export const SHOW_MORE_ACTIONS = i18n.translate('uiActions.showMoreActionsLabel', {
defaultMessage: 'More actions',
});

export const ExtraActionsButton: React.FC<ExtraActionsButtonProps> = ({ onClick, showTooltip }) =>
showTooltip ? (
<EuiToolTip content={SHOW_MORE_ACTIONS}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,14 @@ import {
import React, { useMemo } from 'react';
import { euiThemeVars } from '@kbn/ui-theme';
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
import type { Action } from '../../actions';
import { YOU_ARE_IN_A_DIALOG_CONTAINING_OPTIONS } from './translations';
import { EXTRA_ACTIONS_ARIA_LABEL, YOU_ARE_IN_A_DIALOG_CONTAINING_OPTIONS } from './translations';
import { CellActionExecutionContext } from './cell_actions';

const euiContextMenuItemCSS = css`
color: ${euiThemeVars.euiColorPrimaryText};
`;

const EXTRA_ACTIONS_ARIA_LABEL = i18n.translate('uiActions.cellActions.extraActionsAriaLabel', {
defaultMessage: 'Extra actions',
});

interface ActionsPopOverProps {
actionContext: CellActionExecutionContext;
isOpen: boolean;
Expand All @@ -50,7 +45,7 @@ export const ExtraActionsPopOver: React.FC<ActionsPopOverProps> = ({
closePopover={closePopOver}
panelPaddingSize="xs"
anchorPosition={'downCenter'}
hasArrow={true}
hasArrow
repositionOnScroll
ownFocus
data-test-subj="extraActionsPopOver"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ import { EuiPopover, EuiScreenReaderOnly } from '@elastic/eui';
import React, { useCallback, useMemo, useRef, useState } from 'react';
import { euiThemeVars } from '@kbn/ui-theme';
import { css } from '@emotion/react';
import { i18n } from '@kbn/i18n';
import type { Action } from '../../actions';
import { ActionItem } from './cell_action_item';
import { ExtraActionsButton } from './extra_actions_button';
import { YOU_ARE_IN_A_DIALOG_CONTAINING_OPTIONS } from './translations';
import { ACTIONS_AREA_LABEL, YOU_ARE_IN_A_DIALOG_CONTAINING_OPTIONS } from './translations';
import { partitionActions } from '../hooks/actions';
import { ExtraActionsPopOverWithAnchor } from './extra_actions_popover';
import { CellActionExecutionContext } from './cell_actions';
Expand Down Expand Up @@ -125,9 +124,7 @@ export const HoverActionsPopover = React.memo<Props>(
repositionOnScroll
ownFocus={false}
data-test-subj={'hoverActionsPopover'}
aria-label={i18n.translate('uiActions.cellActions.actionsAriaLabel', {
defaultMessage: 'Actions',
})}
aria-label={ACTIONS_AREA_LABEL}
>
{showHoverContent ? (
<div css={hoverContentWrapperCSS}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,18 @@ export const YOU_ARE_IN_A_DIALOG_CONTAINING_OPTIONS = (fieldName: string) =>
values: { fieldName },
defaultMessage: `You are in a dialog, containing options for field {fieldName}. Press tab to navigate options. Press escape to exit.`,
});

export const EXTRA_ACTIONS_ARIA_LABEL = i18n.translate(
'uiActions.cellActions.extraActionsAriaLabel',
{
defaultMessage: 'Extra actions',
}
);

export const SHOW_MORE_ACTIONS = i18n.translate('uiActions.showMoreActionsLabel', {
defaultMessage: 'More actions',
});

export const ACTIONS_AREA_LABEL = i18n.translate('uiActions.cellActions.actionsAriaLabel', {
defaultMessage: 'Actions',
});

0 comments on commit e6c6a8d

Please sign in to comment.