Add data-test to picker item remove button

[mnezh]

Q SDK Version, OS
0.8.0, 0.8.2 @ OS X

Actual behavior
Prior to 0.8.0 release the following code used to produce a DOM element with data-test="country" property:
ui.picker(name='country', label='Region', values=selected_country, max_choices=1, choices=city_choices)
After 0.8.0, there's no DOM element with data-test="country", which breaks e2e cypress tests

Expected behavior
If UI element has name property defined, it should be translated to DOM element with data-test property with the same value to enable cypress testing

Steps To Reproduce

1. create an app with ui.picker(name='country', ....)
2. run with wave 0.8.x
3. inspect the DOM of the generated UI

[mnezh]

oh, what has changed is that this element is not present in DOM until the parent div is clicked:
<input autocapitalize="off" autocomplete="off" aria-autocomplete="both" spellcheck="false" data-test="country" class="ms-BasePicker-input input-56" aria-controls=" " role="textbox" data-lpignore="true" value="" tabindex="0">

[mturoci]

After a bit of investigation, we came to a conclusion that the problem is that q-peak picker has a default value set and max_choices also to 1. This removes HTML input element (containing data-test attr) which results in failed test. After removing the default item with X button, the HTML input gets rendered correctly as expected.

This means we need to provide a mechanism to allow selecting the remove button for picker items to improve testability.

[vipinnation]

I would like to work on this issue

[mnezh]

@mturoci is it still an issue?

[mturoci]

Go ahead @vipinnation!

@mnezh it's nice to have I would say.

[dbranley]

Hello Wave, I am new to the project and would like to start contributing!

I think that I have a solution for this issue about the data_test attribute for picker when the max_choices threshold is met. In the Fluent documentation for TagPicker, I found removeButtonIconProps, which allow us to add a data_test attribute to the remove button. So I used this to add a new attribute that includes the picker name prefixed with remove_.

The code I added to ui/src/picker.tsx looks like this:

removeButtonIconProps={{ ...removeButtonIconProps, 'data-test' : 'remove_'+m.name } as Fluent.IIconProps}

I added a couple new test cases to picker.test.tsx. I also created a new Cypress test to confirm the remove button can be retrieved with the new attribute and then clicked to remove the selected item. Here is a snipit from that test:

def test_picker(cy):
    cy.visit('/demo')
    cy.locate('picker').type('Ham').type('{enter}')
    cy.locate('picker').should('not.exist')
    cy.locate('remove_picker').first().click()
    cy.locate('picker').should('exist').type('Beans').type('{enter}')
    cy.locate('picker').should('not.exist')

I have a branch in my fork of the project and will be submitting a PR in a bit. Please let me know if you think this is fine or if I'm overlooking something. Thanks!

[mturoci]

Hi @dbranley! Feel free to make a PR. Will have a look and let you know if anything else is needed. Your description sounds reasonable so should be good. Thanks!