Skip to content

Commit

Permalink
Add code review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
machadoum committed Dec 19, 2022
1 parent eb50c58 commit cd96c5e
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default {
(storyFn: Function) => (
<CellActionsContextProvider
// call uiActions getTriggerCompatibleActions(triggerId, data)
getCompatibleActions={getCompatibleActions}
getTriggerCompatibleActions={getCompatibleActions}
>
<div style={{ paddingTop: '70px' }} />
{storyFn()}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('CellActions', () => {
const getActions = () => getActionsPromise;

const { queryByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<CellActions mode={CellActionsMode.ALWAYS_VISIBLE} triggerId={TRIGGER_ID} field={FIELD}>
Field value
</CellActions>
Expand All @@ -39,7 +39,7 @@ describe('CellActions', () => {
const getActions = () => getActionsPromise;

const { queryByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<CellActions mode={CellActionsMode.ALWAYS_VISIBLE} triggerId={TRIGGER_ID} field={FIELD}>
Field value
</CellActions>
Expand All @@ -58,7 +58,7 @@ describe('CellActions', () => {
const getActions = () => getActionsPromise;

const { queryByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<CellActions mode={CellActionsMode.HOVER_POPOVER} triggerId={TRIGGER_ID} field={FIELD}>
Field value
</CellActions>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,3 @@ export const CellActions: React.FC<CellActionsProps> = ({
</div>
);
};

CellActions.displayName = 'CellActions';
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('CellActionsContextProvider', () => {

{
wrapper: ({ children }) => (
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
{children}
</CellActionsContextProvider>
),
Expand Down Expand Up @@ -61,7 +61,7 @@ describe('CellActionsContextProvider', () => {

{
wrapper: ({ children }) => (
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
{children}
</CellActionsContextProvider>
),
Expand All @@ -86,7 +86,7 @@ describe('CellActionsContextProvider', () => {

{
wrapper: ({ children }) => (
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
{children}
</CellActionsContextProvider>
),
Expand All @@ -112,7 +112,7 @@ describe('CellActionsContextProvider', () => {

{
wrapper: ({ children }) => (
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
{children}
</CellActionsContextProvider>
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,69 +7,63 @@
*/

import { orderBy } from 'lodash/fp';
import React, { createContext, FC, useContext, useMemo } from 'react';
import React, { createContext, FC, useCallback, useContext } from 'react';
import useAsync from 'react-use/lib/useAsync';
import useAsyncFn from 'react-use/lib/useAsyncFn';
import type { Action } from '../../actions';
import { CellActionExecutionContext } from './cell_actions';

type GetActionsType = (trigger: string, context: object) => Promise<Action[]>;
// It must to match `UiActionsService.getTriggerCompatibleActions`
type GetTriggerCompatibleActionsType = (triggerId: string, context: object) => Promise<Action[]>;

const initialContext = {
getCompatibleActions: undefined,
};
type GetActionsType = (context: CellActionExecutionContext) => Promise<Action[]>;

const CellActionsContext = createContext<{
getCompatibleActions: GetActionsType | undefined;
}>(initialContext);
const CellActionsContext = createContext<{ getActions: GetActionsType } | null>(null);

interface CellActionsContextProviderProps {
/**
* Please assign `uiActions.getTriggerCompatibleActions` function.
* This function should return a list of actions for a triggerId that are compatible with the provided context.
*/
getCompatibleActions: GetActionsType;
getTriggerCompatibleActions: GetTriggerCompatibleActionsType;
}

export const CellActionsContextProvider: FC<CellActionsContextProviderProps> = ({
children,
getCompatibleActions,
getTriggerCompatibleActions,
}) => {
const getSortedCompatibleActions = useMemo<GetActionsType>(() => {
return (trigger, context) =>
getCompatibleActions(trigger, context).then((actions) =>
const getSortedCompatibleActions = useCallback<GetActionsType>(
(context) =>
getTriggerCompatibleActions(context.trigger.id, context).then((actions) =>
orderBy(['order', 'id'], ['asc', 'asc'], actions)
);
}, [getCompatibleActions]);
),
[getTriggerCompatibleActions]
);

return (
<CellActionsContext.Provider value={{ getCompatibleActions: getSortedCompatibleActions }}>
<CellActionsContext.Provider value={{ getActions: getSortedCompatibleActions }}>
{children}
</CellActionsContext.Provider>
);
};

const useGetCompatibleActions = () => {
const useCellActions = () => {
const context = useContext(CellActionsContext);
if (context.getCompatibleActions === undefined) {
// eslint-disable-next-line no-console
console.error(
if (!context) {
throw new Error(
'No CellActionsContext found. Please wrap the application with CellActionsContextProvider'
);
}

return (cellActionContext: CellActionExecutionContext) =>
context.getCompatibleActions
? context.getCompatibleActions(cellActionContext.trigger.id, cellActionContext)
: Promise.resolve([]);
return context;
};

export const useLoadActions = (context: CellActionExecutionContext) => {
const getCompatibleActions = useGetCompatibleActions();
return useAsync(() => getCompatibleActions(context), []);
const { getActions } = useCellActions();
return useAsync(() => getActions(context), []);
};

export const useLoadActionsFn = () => {
const getCompatibleActions = useGetCompatibleActions();
return useAsyncFn(getCompatibleActions, []);
const { getActions } = useCellActions();
return useAsyncFn(getActions, []);
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,3 @@ export const ExtraActionsButton: React.FC<ExtraActionsButtonProps> = ({ onClick,
onClick={onClick}
/>
);

ExtraActionsButton.displayName = 'ExtraActionsButton';
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ export const ExtraActionsPopOverWithAnchor = ({
) : null;
};

ExtraActionsPopOverWithAnchor.displayName = 'ExtraActionsPopOverWithAnchor';

type ExtraActionsPopOverContentProps = Pick<
ActionsPopOverProps,
'actionContext' | 'closePopOver' | 'actions'
Expand Down Expand Up @@ -133,5 +131,3 @@ const ExtraActionsPopOverContent: React.FC<ExtraActionsPopOverContentProps> = ({
</>
);
};

ExtraActionsPopOverContent.displayName = 'ExtraActionsPopOverContent';
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('HoverActionsPopover', () => {
it('renders', () => {
const getActions = () => Promise.resolve([]);
const { queryByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
children={null}
showMoreActionsFrom={4}
Expand All @@ -42,7 +42,7 @@ describe('HoverActionsPopover', () => {
const getActions = () => getActionsPromise;

const { queryByLabelText, getByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
showMoreActionsFrom={4}
actionContext={actionContext}
Expand All @@ -67,7 +67,7 @@ describe('HoverActionsPopover', () => {
const getActions = () => getActionsPromise;

const { queryByLabelText, getByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
showMoreActionsFrom={4}
actionContext={actionContext}
Expand Down Expand Up @@ -97,7 +97,7 @@ describe('HoverActionsPopover', () => {
const getActions = () => getActionsPromise;

const { getByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
showMoreActionsFrom={1}
actionContext={actionContext}
Expand All @@ -122,7 +122,7 @@ describe('HoverActionsPopover', () => {
const getActions = () => getActionsPromise;

const { getByTestId, getByLabelText } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
showMoreActionsFrom={1}
actionContext={actionContext}
Expand Down Expand Up @@ -156,7 +156,7 @@ describe('HoverActionsPopover', () => {
const getActions = () => getActionsPromise;

const { getByTestId, queryByLabelText } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<HoverActionsPopover
showMoreActionsFrom={2}
actionContext={actionContext}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,5 +160,3 @@ export const HoverActionsPopover = React.memo<Props>(
);
}
);

HoverActionsPopover.displayName = 'HoverActionsPopover';
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('InlineActions', () => {
const getActionsPromise = Promise.resolve([]);
const getActions = () => getActionsPromise;
const { queryByTestId } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<InlineActions showMoreActionsFrom={5} actionContext={actionContext} showTooltip={false} />
</CellActionsContextProvider>
);
Expand All @@ -41,7 +41,7 @@ describe('InlineActions', () => {
]);
const getActions = () => getActionsPromise;
const { queryAllByRole } = render(
<CellActionsContextProvider getCompatibleActions={getActions}>
<CellActionsContextProvider getTriggerCompatibleActions={getActions}>
<InlineActions showMoreActionsFrom={5} actionContext={actionContext} showTooltip={false} />
</CellActionsContextProvider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,3 @@ export const InlineActions: React.FC<InlineActionsProps> = ({
</span>
);
};

InlineActions.displayName = 'InlineActions';

0 comments on commit cd96c5e

Please sign in to comment.