Skip to content

Commit

Permalink
fix(APP-3806): Fix warning feedback on ProposalAction component, disa…
Browse files Browse the repository at this point in the history
…ble decoded view for actions with no parameters (#366)
  • Loading branch information
cgero-eth authored Dec 11, 2024
1 parent 092c2ef commit cc8c8c3
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix color of body name on `<ProposalVoting.BodyContext />` module component
- Fix re-render issue on `<AddressInput />` module component when `onAccept` prop changes on every render
- Update `<CheckboxCard />` core component to set full-width to `children` property wrapper
- Update `<ProposalActions.Item />` module component to display correct feedback for actions with data and value and
disable decoded view for actions without parameters

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ const meta: Meta<typeof ProposalActions.Item> = {
type Story = StoryObj<typeof ProposalActions.Item>;

/**
* For verified actions, the ProposalActions.Item component renders the DECODED view by default and disables the basic view.
* For verified actions with parameters, the ProposalActions.Item component renders the DECODED view by default and disables the BASIC view.
*/
export const Verified: Story = {
export const VerifiedDecoded: Story = {
render: defaultRender,
args: {
index: 0,
Expand Down Expand Up @@ -82,7 +82,27 @@ export const Verified: Story = {
};

/**
* For unverified actions, the ProposalActions.Item component renders the RAW view by default and disables the decoded and basic views.
* For verified actions without parameters, the ProposalActions.Item component renders the RAW view by default and disables the DECODED and BASIC views.
*/
export const VerifiedRaw: Story = {
render: defaultRender,
args: {
index: 0,
action: generateProposalAction({
value: '100000000000',
data: '0x',
to: '0x767f4616E322e36AF4d2d63f0b35c256545b25C9',
inputData: {
function: 'depositETH',
contract: 'UniBridge',
parameters: [],
},
}),
},
};

/**
* For unverified actions, the ProposalActions.Item component renders the RAW view by default and disables the DECODED and BASIC views.
*/
export const Unverified: Story = {
render: defaultRender,
Expand Down Expand Up @@ -135,9 +155,9 @@ export const Native: Story = {
};

/**
* Actions are marked as critical if they are not native transfers but have a value greater than 0.
* Actions are displayed with a warning feedback if they are not native transfers but have a value greater than 0.
*/
export const Critical: Story = {
export const Warning: Story = {
render: defaultRender,
args: {
index: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,21 @@ describe('<ProposalActionsItem /> component', () => {
expect(actionDecoder.dataset.mode).toEqual(ProposalActionsDecoderMode.READ);
});

it('defaults the view-mode to decoded and read mode when action has no custom component and is verified', async () => {
it('defaults the view-mode to raw and read mode when action has no custom component, is verified but has no parameters', async () => {
const action = generateProposalAction({ inputData: { function: '', contract: '', parameters: [] } });
render(createTestComponent({ action }));
await userEvent.click(screen.getByRole('button'));
const actionDecoder = screen.getByTestId('decoder-mock');
expect(actionDecoder.dataset.view).toEqual(ProposalActionsDecoderView.RAW);
expect(actionDecoder.dataset.mode).toEqual(ProposalActionsDecoderMode.READ);
});

it('defaults the view-mode to decoded and read mode when action has no custom component, is verified and has parameters', async () => {
const params = [{ name: 'test', type: 'uint', value: '' }];
const action = generateProposalAction({ inputData: { function: '', contract: '', parameters: params } });
render(createTestComponent({ action }));
await userEvent.click(screen.getByRole('button'));
const actionDecoder = screen.getByTestId('decoder-mock');
expect(actionDecoder.dataset.view).toEqual(ProposalActionsDecoderView.DECODED);
expect(actionDecoder.dataset.mode).toEqual(ProposalActionsDecoderMode.READ);
});
Expand All @@ -140,17 +150,18 @@ describe('<ProposalActionsItem /> component', () => {
expect(screen.getByTestId(IconType.WARNING)).toBeInTheDocument();
});

it('renders a critical icon and alert when action value is not zero and action is not a native transfer', async () => {
it('renders a warning icon and alert when action value is not zero and action is not a native transfer', async () => {
const action = generateProposalAction({ value: '1000000000000000000', data: '0xabc' });
render(createTestComponent({ action }));
expect(screen.getByTestId(IconType.CRITICAL)).toBeInTheDocument();
expect(screen.getByTestId(IconType.WARNING)).toBeInTheDocument();
await userEvent.click(screen.getByRole('button'));
expect(screen.getByText(modulesCopy.proposalActionsItem.nativeSendAlert)).toBeInTheDocument();
expect(screen.getByText(modulesCopy.proposalActionsItem.nativeSendDescription('1')));
});

it('updates active view on view-mode change', async () => {
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: [] } });
const params = [{ name: 'amount', type: 'uint', value: null }];
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: params } });
render(createTestComponent({ action }));
await userEvent.click(screen.getByRole('button'));
expect(screen.getByTestId('decoder-mock').dataset.view).toEqual(ProposalActionsDecoderView.DECODED);
Expand Down Expand Up @@ -190,15 +201,17 @@ describe('<ProposalActionsItem /> component', () => {
});

it('renders the decoded-view in edit mode when editMode prop is true and action does not support basic view', () => {
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: [] } });
const params = [{ name: 'param', type: 'address', value: '' }];
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: params } });
isActionSupportedSpy.mockReturnValue(false);
render(createTestComponent({ action, editMode: true }));
expect(screen.getByTestId('decoder-mock').dataset.view).toEqual(ProposalActionsDecoderView.DECODED);
expect(screen.getByTestId('decoder-mock').dataset.mode).toEqual(ProposalActionsDecoderMode.EDIT);
});

it('renders the decoded-view in watch mode when editMode prop is true and action supports basic view', async () => {
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: [] } });
const params = [{ name: 'amount', type: 'uint', value: null }];
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: params } });
isActionSupportedSpy.mockReturnValue(true);
render(createTestComponent({ action, editMode: true }));
await userEvent.click(screen.getByRole('button', { name: modulesCopy.proposalActionsItem.menu.dropdownLabel }));
Expand All @@ -215,7 +228,8 @@ describe('<ProposalActionsItem /> component', () => {
});

it('renders the raw-view in watch mode when editMode prop is true and action supports decoded view', async () => {
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: [] } });
const params = [{ name: 'amount', type: 'uint', value: null }];
const action = generateProposalAction({ inputData: { contract: '', function: '', parameters: params } });
isActionSupportedSpy.mockReturnValue(true);
render(createTestComponent({ action, editMode: true }));
await userEvent.click(screen.getByRole('button', { name: modulesCopy.proposalActionsItem.menu.dropdownLabel }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,14 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
const itemRef = useRef<HTMLDivElement>(null);

const supportsBasicView = CustomComponent != null || proposalActionsItemUtils.isActionSupported(action);

const isAbiAvailable = action.inputData != null;
const supportsDecodedView = isAbiAvailable && action.inputData?.parameters.length;

const [activeViewMode, setActiveViewMode] = useState<ProposalActionsItemViewMode>(
supportsBasicView
? 'BASIC'
: isAbiAvailable
: supportsDecodedView
? ProposalActionsDecoderView.DECODED
: ProposalActionsDecoderView.RAW,
);
Expand All @@ -51,25 +53,18 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
const displayValueWarning = action.value !== '0' && action.data !== '0x';
const formattedValue = formatUnits(BigInt(action.value), 18);

const headerIcon = displayValueWarning
? { icon: IconType.CRITICAL, className: 'text-critical-500' }
: { icon: IconType.WARNING, className: 'text-warning-500' };

const functionNameStyle = displayValueWarning
? 'text-critical-800'
: !isAbiAvailable
? 'text-warning-800'
: 'text-neutral-800';
const displayWarningFeedback = displayValueWarning || !isAbiAvailable;
const functionNameStyle = displayWarningFeedback ? 'text-warning-800' : 'text-neutral-800';

const viewModes = [
{ mode: 'BASIC' as const, disabled: !supportsBasicView },
{ mode: ProposalActionsDecoderView.DECODED, disabled: !isAbiAvailable },
{ mode: ProposalActionsDecoderView.DECODED, disabled: !supportsDecodedView },
{ mode: ProposalActionsDecoderView.RAW },
];

const { EDIT, WATCH, READ } = ProposalActionsDecoderMode;
const decodedViewMode = editMode && !supportsBasicView ? EDIT : editMode ? WATCH : READ;
const rawViewMode = editMode && !isAbiAvailable ? EDIT : editMode ? WATCH : READ;
const rawViewMode = editMode && !supportsDecodedView ? EDIT : editMode ? WATCH : READ;

return (
<Accordion.Item value={index.toString()} ref={itemRef}>
Expand All @@ -79,8 +74,8 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
<p className={classNames('text-base font-normal leading-tight md:text-lg', functionNameStyle)}>
{action.inputData?.function ?? copy.proposalActionsItem.notVerified.function}
</p>
{(!isAbiAvailable || displayValueWarning) && (
<Icon icon={headerIcon.icon} size="md" className={headerIcon.className} />
{displayWarningFeedback && (
<Icon icon={IconType.WARNING} size="md" className="text-warning-500" />
)}
</div>
<div className="flex items-center gap-2 md:gap-3">
Expand All @@ -97,7 +92,7 @@ export const ProposalActionsItem = <TAction extends IProposalAction = IProposalA
<div className="flex flex-col items-start gap-y-6 self-start md:gap-y-8">
{displayValueWarning && (
<AlertCard
variant="critical"
variant="warning"
message={copy.proposalActionsItem.nativeSendAlert}
description={copy.proposalActionsItem.nativeSendDescription(formattedValue)}
/>
Expand Down

0 comments on commit cc8c8c3

Please sign in to comment.