Skip to content

Commit

Permalink
fix: use first dropdown option as the default value (#2465)
Browse files Browse the repository at this point in the history
Fixes a problem where the default state for a dropdown would be `null`
even though the UI would show option 1 as selected.

We fix this by using option 1 as the default state value if nothing is
specified.
  • Loading branch information
FrederikBolding committed Jun 6, 2024
1 parent 0c166a9 commit ef93601
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 5 deletions.
4 changes: 3 additions & 1 deletion packages/examples/packages/interactive-ui/src/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ describe('onRpcRequest', () => {
const resultScreen = await response.getInterface();

expect(resultScreen).toRender(
<Result values={{ 'example-input': '', 'example-dropdown': '' }} />,
<Result
values={{ 'example-input': '', 'example-dropdown': 'option1' }}
/>,
);
await resultScreen.ok();

Expand Down
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 91.59,
"functions": 96.76,
"branches": 91.64,
"functions": 96.77,
"lines": 97.89,
"statements": 97.57
}
36 changes: 36 additions & 0 deletions packages/snaps-controllers/src/interface/utils.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,22 @@ describe('constructState', () => {
});
});

it('sets default value for root level dropdown', () => {
const element = (
<Box>
<Dropdown name="foo">
<Option value="option1">Option 1</Option>
<Option value="option2">Option 2</Option>
</Dropdown>
</Box>
);

const result = constructState({}, element);
expect(result).toStrictEqual({
foo: 'option1',
});
});

it('supports root level dropdowns', () => {
const element = (
<Box>
Expand All @@ -248,6 +264,26 @@ describe('constructState', () => {
});
});

it('sets default value for dropdowns in forms', () => {
const element = (
<Box>
<Form name="form">
<Field label="foo">
<Dropdown name="foo">
<Option value="option1">Option 1</Option>
<Option value="option2">Option 2</Option>
</Dropdown>
</Field>
</Form>
</Box>
);

const result = constructState({}, element);
expect(result).toStrictEqual({
form: { foo: 'option1' },
});
});

it('supports dropdowns in forms', () => {
const element = (
<Box>
Expand Down
35 changes: 33 additions & 2 deletions packages/snaps-controllers/src/interface/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
FieldElement,
InputElement,
JSXElement,
OptionElement,
} from '@metamask/snaps-sdk/jsx';
import { isJSXElementUnsafe } from '@metamask/snaps-sdk/jsx';
import {
Expand Down Expand Up @@ -48,6 +49,26 @@ export function assertNameIsUnique(state: InterfaceState, name: string) {
);
}

/**
* Construct default state for a component.
*
* This function is meant to be used inside constructInputState to account
* for component specific defaults and will not override the component value or existing form state.
*
* @param element - The input element.
* @returns The default state for the specific component, if any.
*/
function constructComponentSpecificDefaultState(
element: InputElement | DropdownElement,
) {
if (element.type === 'Dropdown') {
const children = getJsxChildren(element) as OptionElement[];
return children[0].props.value;
}

return null;
}

/**
* Construct the state for an input field.
*
Expand All @@ -59,7 +80,12 @@ function constructInputState(
oldState: InterfaceState,
element: InputElement | DropdownElement,
) {
return element.props.value ?? oldState[element.props.name] ?? null;
return (
element.props.value ??
oldState[element.props.name] ??
constructComponentSpecificDefaultState(element) ??
null
);
}

/**
Expand All @@ -77,7 +103,12 @@ function constructFormInputState(
) {
const oldFormState = oldState[form] as FormState;
const oldInputState = oldFormState?.[component.props.name];
return component.props.value ?? oldInputState ?? null;
return (
component.props.value ??
oldInputState ??
constructComponentSpecificDefaultState(component) ??
null
);
}

/**
Expand Down

0 comments on commit ef93601

Please sign in to comment.