[WNMGDS-2723] Choice web component #3047
Conversation
| return ( | ||
| <ds-choice {...args}> | ||
| <div slot="checked-children"> | ||
| <div className="ds-c-alert">{args['checked-children']}</div> |
There was a problem hiding this comment.
for some reason hooking the story up to the args caused the html to be stripped away. if the value is hard-coded it's fine, so this seems like a storybook problem.
There was a problem hiding this comment.
Update: @zarahzachz wrote a follow-up ticket for adding partial attribute pass-through support.
packages/design-system/src/components/web-components/ds-choice/ds-choice.test.tsx
Show resolved
Hide resolved
zarahzachz
added
Impacts: Core
| const Template = (args) => { | ||
| useEffect(() => { | ||
| const onChange = (event) => { | ||
| action('ds-change')(event); |
There was a problem hiding this comment.
I guess the Storybook action output is too shallow to show all the properties for event.detail.target, but I made sure that event.detail.target.checked changed when checking and unchecking.
| <code>event.details.target.value</code> - The <code>value</code> of the selected | ||
| option |
There was a problem hiding this comment.
This should also reference event.detail.target.checked because that's probably the most relevant piece of information
| decorators: [webComponentDecorator], | ||
| }; | ||
|
|
||
| const Template = (args) => { |
There was a problem hiding this comment.
The checked-children and unchecked-children attributes should be destructured from this so they don't also get spread onto the ds-choice element itself.
There was a problem hiding this comment.
When it gets removed from the attributes on the element, the alert divs start showing up correctly
| describe('Button', () => { | ||
| describe('Dropdown', () => { |
There was a problem hiding this comment.
Need to update the snapshots after this change
| describe('event handlers and emitters', () => { | ||
| let props; | ||
|
|
||
| beforeEach(() => { | ||
| props = { | ||
| onBlur: jest.fn(), | ||
| onChange: jest.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| it('calls the onChange handler', () => { | ||
| renderChoice(props); | ||
| const el = screen.getByRole('checkbox'); | ||
| userEvent.click(el); | ||
|
|
||
| expect(props.onBlur).toHaveBeenCalledTimes(0); | ||
| expect(props.onChange).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| it('calls the onBlur handler', () => { | ||
| renderChoice(props); | ||
| const el = screen.getByLabelText('George Washington'); | ||
| el.focus(); | ||
| userEvent.tab(); | ||
|
|
||
| expect(props.onBlur).toHaveBeenCalledTimes(1); | ||
| expect(props.onChange).toHaveBeenCalledTimes(0); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
This needs to be updated to use addEventListener and the custom events. I think these are only being passed down successfully by a trick of Preact in the unit tests.
| <div slot="checked-children"> | ||
| <strong data-testid="checked" className="checked-child"> | ||
| I am checked | ||
| </strong> | ||
| </div> | ||
| <div slot="unchecked-children"> | ||
| <strong data-testid="unchecked" className="unchecked-child"> | ||
| I am unchecked | ||
| </strong> | ||
| </div> |
There was a problem hiding this comment.
Since we're adding both, maybe we should do a check for internal state management in this unit test where we click on the checkbox and see if the other one becomes visible and the first one gets hidden.
There was a problem hiding this comment.
Oh, I now see that's what you were doing in the next test
| return ( | ||
| <ds-choice {...args}> | ||
| <div slot="checked-children"> | ||
| <div className="ds-c-alert">{args['checked-children']}</div> |
There was a problem hiding this comment.
Update: @zarahzachz wrote a follow-up ticket for adding partial attribute pass-through support.
| const input = screen.getByRole('checkbox'); | ||
| userEvent.click(input); | ||
|
|
||
| expect(mockHandler).toHaveBeenCalledTimes(1); |
There was a problem hiding this comment.
I wanted to assert here that mockHandler.mock.lastCall[0].details.target.checked was true, but the event object is kind of messed up. I'm wondering if it's a limitation with jsdom and custom events. It seems to work right in Storybook.
* init choice web component. * WIP checked children implementation * init tests. * export choice, add inversed prop, slots. * add choice to astro wc * add conditional rendering of slots * add conditional rendering of slots for astro * get choice examples working * fix typo in dropdown test. * implement choice unit tests * remove unneeded logic from storybook * Update unit test snapshot * Apply Storybook feedback * Add missing attributes in choice and dropdown * Update storybook doc snapshots * Update VRT refs * Make some quasi-realistic use-case scenario for the checkbox in the example * Update examples VRT refs * Updated unit tests * Order of these changed --------- Co-authored-by: Patrick Wolfert <[email protected]>
WNMGDS-2723
Implement Choice as a web component.
checkedChildrenanduncheckedChildrenare being used as slots instead of proper attributes and that's OK!