Fix: UrlInput autoFocus Issue With Custom Block

[yahilmadakiya]

Fix: #6196

Added option to set autofocus true/false. Default can be true.

[danielbachhuber]

Thanks @yahilmadakiya ! Can you include unit tests for this change?

[yahilmadakiya]

Sure @danielbachhuber, I'll try to add unit test.

[yahilmadakiya]

@danielbachhuber I have tried a unit test for the changes.

On my research, I came to know that as of now enzyme not providing an option to test for :focus.
Issue link

My unit test 👇

it( 'should not have autofocus if autofocus is false', () => {
	const wrapper = mount( <UrlInputButton autofocus={ false } /> );
	wrapper.setState( { expanded: true } );
	const elem = wrapper.find( 'input[type="text"]' );
	expect( elem.is( ':focus' ) ).toBe( false );
} );

[danielbachhuber]

@yahilmadakiya Looks fine to me. Can you go ahead and commit the unit test?

[yahilmadakiya]

@danielbachhuber Above test throw error
Enzyme::Selector does not support pseudo-element or pseudo-class selectors.

Please check my comment here 👉 enzymejs/enzyme#703 (comment)

[aduth]

This is a good use case for destructuring default assignment, just as we're doing with value:

const { value = '', autoFocus = true, instanceId } = this.props;

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Default_values_2

[yahilmadakiya]

@aduth Thanks.

[aduth]

For your test, could this work?

it( 'should not have autofocus if autofocus is false', () => {
	const wrapper = mount( <UrlInputButton autofocus={ false } /> );
	wrapper.setState( { expanded: true } );
	const elem = wrapper.find( 'input[type="text"]' ).getDOMNode();
	expect( document.activeElement ).toBe( elem );
} );

(I've not tried it, and not sure if JSDOM is capable of assigning activeElement by autofocus)

[yahilmadakiya]

@aduth I have already tried with activeElement. It was not working as per expectation.

Also, I have tried with the above test which you have provided. The Unit test passed in both case autofocus={ true } and autofocus={ false }.

Thought: activeElement has Working Draft status. So, I dont think we should use that.
https://developer.mozilla.org/en-US/docs/Web/API/DocumentOrShadowRoot/activeElement

[aduth]

That MDN link is unfortunately a bit confusing. I think the working draft status is referring to shadow DOM specifically. It is part of the WHATWG standard:

https://html.spec.whatwg.org/multipage/interaction.html#dom-document-activeelement

The Unit test passed in both case autofocus={ true } and autofocus={ false }.

Could it be that this needs to be corrected to the camel-case form autoFocus ?

[danielbachhuber]

Given the difficulty of testing this, I think it's fine to land without tests.

[danielbachhuber]

Rebased with master and force-pushed to resolve merge conflict.