Fixes Select Component initial value is always the first option

[iuravic]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

As detected by issue #83 , "... when using the SelectControl component, the initial value for the select element upon page load uses the value of the first child option element, rather than having no - or an empty - value".

Here this is fixed by adding a first disabled option with the label "- Select -" and empty value.

However, since the SelectControl as BaseComponent from'@wordpress/components doesn't support the disabled attribute for the <option> element (see in codebase), now switched to using a manual <select> HTML element.

This PR should eventually update the BaseComponent, but since a PR process could be relatively lengthy, and our publishers are experiencing the bug right now, currently switched the component to use a pure <select> element with a <label>.

Also as a minor change, in the Components Demo wizard, updated more meaningful labels and fixed the wrong use of onChange to handle event as argument, rather than value.

When now clicking on the Select component (specifically relevant for a Select with no previous selections), a first disabled option "- Select -" is present, enabling correct onClick of the first option:

Closes #83 .

How to test the changes in this Pull Request:

1. Pull and build nvm use 10.10.0 && npm ci && npm run clean && npm run build:webpack -- --watch.
2. Visit the Components demo page at /wp-admin/admin.php?page=newspack-components-demo.
3. Verify that the three demoed Select Components are initially visually sound.
4. Check that the second Select component with label "Label for Select with no selection" now accepts selection of the first option with a single click, and that on clicking away the selection persists/sticks.
5. Test click the remaining two Select components.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[jeffersonrabb]

I'd like to propose a different short-term solution to this that assumes this line will eventually find its way into @wordpress/components:

1. Within the Newspack component's src directory, duplicate the Gutenberg SelectControl source. You'll need to change the BaseControl import but other than that it should be clean.
2. Change the SelectControl import to grab your copy rather than the original.
3. Add support for the disabled attribute to the copied code.
4. Make the changes to assets/components/src/select-control/index.js as you would have originally had the disabled support been available.

If and when the proposed change is accepted in @wordpress/components we'll simply delete the copy and revert the SelectControl import.

A couple of other comments:

1. The text of the disabled first option should be settable via a prop (maybe placeholderOptionLabel?)
2. Including this option should be optional. I'd probably suggest that the option is included only if the text is explicitly set.

[iuravic]

Thanks for all the comments! Great thinking! 👍I agree, that's a smarter solution, since it will make switching to the updated @wordpress/components a breeze.

To handle both your comments, I made the disabled 1st option just a regular option with the disabled attribute. This seems simplest since it allows to control all these things, and it's the same way the PR is handling it.

It would be great if you could please give it another test run.

[jeffersonrabb]

It looks like you might be using a compiled file rather than the src? Try using this one instead:

https://github.com/WordPress/gutenberg/blob/master/packages/components/src/select-control/index.js#L51

You'll need to make only a couple of changes to it:

1. Import BaseControl from @wordpress/components instead of locally:

import { BaseControl } from '@wordpress/components';

2. Add an export at the end:

export default withInstanceId( SelectControl );

and of course, add the disabled attribute handling.

The only downside is we're losing the Gutenberg component's styling (https://github.com/WordPress/gutenberg/blob/master/packages/components/src/select-control/style.scss), which would be a little tricky to import since it uses the break-medium mixin.

I would also suggest putting it in a subdirectory of assets/components/src/select-control. so there isn't any confusion about it being a new Component meant for use.

[iuravic]

Thank you for noticing all this!
Now updated.

[jeffersonrabb]

Looks good, thanks for doing this! One last request—could you also run the stylesheet through Prettier? That will also catch a couple of syntactical odds and ends. Once you've done this, 🚢!

[iuravic]

@jeffersonrabb thank you so much for your patience! Reformatted the stylesheet as well, and it's up to the formatting standards now. Merging!