Add tests to withFocusReturn HOC. #931
Conversation
BE-Webdesign
added
the
[Type] Automated Testing
| expect( wrappedElementShallow.text() ).to.equal( 'Testing' ); | ||
| } ); | ||
|
|
||
| it( 'passing additonal props', () => { |
|
I'd clean up the test assertion messages a bit, as I'll note inline next to each one. |
| } | ||
|
|
||
| describe( 'withFocusReturn()', () => { | ||
| describe( 'expected behavior', () => { |
There was a problem hiding this comment.
Do we need this second describe block? Our tests are all testing "expected behavior" after all.
There was a problem hiding this comment.
I do see that the other PRs include multiple 2nd-level describe blocks though, so this is probably fine.
There was a problem hiding this comment.
Yeah, I tried to mirror other examples as best as I could. Probably did a bad job lol.
| activeElement.blur(); | ||
| } ); | ||
|
|
||
| it( 'rendering with a basic <div> element', () => { |
| expect( wrappedElement.node.props.test ).to.equal( 'test' ); | ||
| } ); | ||
|
|
||
| it( 'when component mounts and unmounts', () => { |
There was a problem hiding this comment.
it should do [something] when component mounts and unmounts
| expect( document.activeElement ).to.equal( switchFocusTo ); | ||
| } ); | ||
|
|
||
| it( 'should return focus to element associated with HOC', () => { |
| @@ -23,7 +23,7 @@ function withFocusReturn( WrappedComponent ) { | |||
| if ( | |||
| this.activeElement && ( | |||
| ( document.activeElement && wrapper && wrapper.contains( document.activeElement ) ) || | |||
| ! document.activeElement | |||
| ( ! document.activeElement || document.body === document.activeElement ) | |||
There was a problem hiding this comment.
In this case I wonder if we need ! document.activeElement at all?
There was a problem hiding this comment.
Yeah, there might be some possibility of this existing somehow, at one point JSDOM would not handle this to the HTML spec, so ! document.activeElement would be falsy. I think keeping it in is okay. Adds to the readability a bit in my opinion as well.
Adds basic withFocusReturn HOCtests. Covers cases dealing with the handling of the focus. Related to progress on #641. Adds an extra conditional to match if the body is focused the active elment should become the one bound to the HOC, this is necessary as in the HTML DOM spec it is not supposed to be possible to have `! document.activeElement` ever be true. Testing Instructions Run npm i && npm run test-unit ensure tests pass. Change Dashicon logic to ensure tests fail as they should. Verify disposable focus returning still works.
BE-Webdesign
force-pushed
the
add/test/components/higher-order
branch
from
861e7c0 to
1e4ff65
Compare
|
Updated this. Let me know what you think of the conventions now. |
Adds basic withFocusReturn HOCtests. Covers cases dealing with the
handling of the focus. Related to progress on #641. Adds an extra
conditional to match if the body is focused the active elment should
become the one bound to the HOC, this is necessary as in the HTML DOM
spec it is not supposed to be possible to have
! document.activeElementever be true.Testing Instructions
Run npm i && npm run test-unit ensure tests pass. Change Dashicon logic
to ensure tests fail as they should. Verify disposable focus returning
still works.