From 09873d8450e8fab122f44f9710e14cfa46abebd9 Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Wed, 14 Mar 2018 18:47:06 -0700 Subject: [PATCH 01/10] Revert "FocusTrapZone does not correctly trap focus when last child is FocusZone (#4172)" This reverts commit 699fa697ae829e756108ce2b6725084d27e93f7e. --- .../FocusTrapZone/FocusTrapZone.test.tsx | 71 ------------------- packages/utilities/src/focus.ts | 17 +++-- 2 files changed, 8 insertions(+), 80 deletions(-) diff --git a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx index 9599f02be4c3d1..d23d8fc60e7e17 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx @@ -239,75 +239,4 @@ describe('FocusTrapZone', () => { ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab }); expect(lastFocusedElement).toBe(buttonX); }); - - it('can tab across FocusZones with different button structures', () => { - const component = ReactTestUtils.renderIntoDocument( -
- - - - - - - - -
- ); - - const focusTrapZone = ReactDOM.findDOMNode(component as React.ReactInstance) as Element; - - const buttonA = focusTrapZone.querySelector('.a') as HTMLElement; - const buttonD = focusTrapZone.querySelector('.d') as HTMLElement; - const buttonE = focusTrapZone.querySelector('.e') as HTMLElement; - const buttonF = focusTrapZone.querySelector('.f') as HTMLElement; - - // Assign bounding locations to buttons. - setupElement(buttonA, { - clientRect: { - top: 0, - bottom: 30, - left: 0, - right: 30 - } - }); - - setupElement(buttonD, { - clientRect: { - top: 30, - bottom: 60, - left: 0, - right: 30 - } - }); - - setupElement(buttonE, { - clientRect: { - top: 30, - bottom: 60, - left: 30, - right: 60 - } - }); - - setupElement(buttonF, { - clientRect: { - top: 30, - bottom: 60, - left: 60, - right: 90 - } - }); - - // Focus the first button. - ReactTestUtils.Simulate.focus(buttonD); - expect(lastFocusedElement).toBe(buttonD); - - // Pressing tab should go to a. - ReactTestUtils.Simulate.keyDown(buttonD, { which: KeyCodes.tab }); - expect(lastFocusedElement).toBe(buttonA); - - // Pressing shift + tab should go to a. - ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab, shiftKey: true }); - expect(lastFocusedElement).toBe(buttonD); - }); }); diff --git a/packages/utilities/src/focus.ts b/packages/utilities/src/focus.ts index e3f47ce9bd641b..c2dc853c6932cb 100644 --- a/packages/utilities/src/focus.ts +++ b/packages/utilities/src/focus.ts @@ -77,7 +77,7 @@ export function getPreviousElement( traverseChildren?: boolean, includeElementsInFocusZones?: boolean, allowFocusRoot?: boolean, - elementShouldBeTabbable?: boolean): HTMLElement | null { + tabbable?: boolean): HTMLElement | null { if (!currentElement || (!allowFocusRoot && currentElement === rootElement)) { @@ -97,10 +97,10 @@ export function getPreviousElement( true, includeElementsInFocusZones, allowFocusRoot, - elementShouldBeTabbable); + tabbable); if (childMatch) { - if ((elementShouldBeTabbable && isElementTabbable(childMatch, true)) || !elementShouldBeTabbable) { + if ((tabbable && (isElementTabbable(childMatch, true))) || !tabbable) { return childMatch; } @@ -112,7 +112,7 @@ export function getPreviousElement( true, includeElementsInFocusZones, allowFocusRoot, - elementShouldBeTabbable + tabbable ); if (childMatchSiblingMatch) { return childMatchSiblingMatch; @@ -133,7 +133,7 @@ export function getPreviousElement( true, includeElementsInFocusZones, allowFocusRoot, - elementShouldBeTabbable + tabbable ); if (childMatchParentMatch) { @@ -146,8 +146,7 @@ export function getPreviousElement( } // Check the current node, if it's not the first traversal. - if (checkNode && isCurrentElementVisible && - ((elementShouldBeTabbable && isElementTabbable(currentElement, true)) || !elementShouldBeTabbable)) { + if (checkNode && isCurrentElementVisible && isElementTabbable(currentElement)) { return currentElement; } @@ -160,7 +159,7 @@ export function getPreviousElement( true, includeElementsInFocusZones, allowFocusRoot, - elementShouldBeTabbable); + tabbable); if (siblingMatch) { return siblingMatch; @@ -169,7 +168,7 @@ export function getPreviousElement( // Check its parent. if (!suppressParentTraversal) { return getPreviousElement(rootElement, currentElement.parentElement, true, false, false, includeElementsInFocusZones, - allowFocusRoot, elementShouldBeTabbable); + allowFocusRoot, tabbable); } return null; From ff19db28d4b14df4a11eae6a9aa1b6c570199696 Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Thu, 29 Mar 2018 16:29:31 -0700 Subject: [PATCH 02/10] fix picker suggestions and add many tests --- .../src/stories/PeoplePicker.stories.tsx | 66 ++++++++++++++++++- .../pickers/Suggestions/Suggestions.tsx | 2 +- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index 799c7efd2b5c09..cca39588249d15 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -47,6 +47,56 @@ const people: (IPersonaProps & { key: string | number })[] = [ tertiaryText: 'In a meeting', optionalText: 'Available at 4:00pm', presence: PersonaPresence.offline + }, + { + key: 5, + imageUrl: TestImages.personaMale, + imageInitials: 'RK', + primaryText: 'Alex Lundberg1', + secondaryText: 'Financial Analyst', + tertiaryText: 'In a meeting', + optionalText: 'Available at 4:00pm', + presence: PersonaPresence.offline + }, + { + key: 6, + imageUrl: TestImages.personaMale, + imageInitials: 'RK', + primaryText: 'Alex Lundberg2', + secondaryText: 'Financial Analyst', + tertiaryText: 'In a meeting', + optionalText: 'Available at 4:00pm', + presence: PersonaPresence.offline + }, + { + key: 7, + imageUrl: TestImages.personaMale, + imageInitials: 'RK', + primaryText: 'Alex Lundberg2', + secondaryText: 'Financial Analyst', + tertiaryText: 'In a meeting', + optionalText: 'Available at 4:00pm', + presence: PersonaPresence.offline + }, + { + key: 8, + imageUrl: TestImages.personaMale, + imageInitials: 'RK', + primaryText: 'Alex Lundberg3', + secondaryText: 'Financial Analyst', + tertiaryText: 'In a meeting', + optionalText: 'Available at 4:00pm', + presence: PersonaPresence.offline + }, + { + key: 9, + imageUrl: TestImages.personaMale, + imageInitials: 'RK', + primaryText: 'Alex Lundberg4', + secondaryText: 'Financial Analyst', + tertiaryText: 'In a meeting', + optionalText: 'Available at 4:00pm', + presence: PersonaPresence.offline } ]; @@ -96,7 +146,21 @@ storiesOf('PeoplePicker', module) pickerSuggestionsProps={ suggestionProps } disabled /> - )) + )).add('Normal with text', () => ( + + + )) .addDecorator(story => ( extends BaseComponent, ISuggest > { suggestions.map((suggestion, index) =>
Date: Thu, 29 Mar 2018 16:29:38 -0700 Subject: [PATCH 03/10] adding snap file --- .../pickers/Suggestions/Suggestions.test.tsx | 91 + .../__snapshots__/Suggestions.test.tsx.snap | 3095 +++++++++++++++++ 2 files changed, 3186 insertions(+) create mode 100644 packages/office-ui-fabric-react/src/components/pickers/Suggestions/Suggestions.test.tsx create mode 100644 packages/office-ui-fabric-react/src/components/pickers/Suggestions/__snapshots__/Suggestions.test.tsx.snap diff --git a/packages/office-ui-fabric-react/src/components/pickers/Suggestions/Suggestions.test.tsx b/packages/office-ui-fabric-react/src/components/pickers/Suggestions/Suggestions.test.tsx new file mode 100644 index 00000000000000..2f370e4ef271c5 --- /dev/null +++ b/packages/office-ui-fabric-react/src/components/pickers/Suggestions/Suggestions.test.tsx @@ -0,0 +1,91 @@ + +/* tslint:disable:no-unused-variable */ +import * as React from 'react'; +import * as ReactDOM from 'react-dom'; +import * as ReactTestUtils from 'react-dom/test-utils'; +/* tslint:enable:no-unused-variable */ +import * as renderer from 'react-test-renderer'; + +import { Suggestions } from './Suggestions'; +import { ISuggestionItemProps } from './Suggestions.types'; +import { ISuggestionModel } from './SuggestionsController'; + +const suggestions = [ + 'black', + 'blue', + 'brown', + 'cyan', + 'green', + 'magenta', + 'mauve', + 'orange', + 'pink', + 'purple', + 'red', + 'rose', + 'violet', + 'white', + 'yellow']; + +function generateSimpleSuggestions(selectedIndex: number = 0) { + return suggestions.map>((value, index) => { + return { + item: { + key: value, + name: value + }, + selected: index === 0 + }; + }); + +} + +const basicSuggestionRenderer = (props: ISimple) => { + return
{ props.name }
; +}; + +export interface ISimple { + key: string; + name: string; +} + +function mockOnClick() { + console.log('clicked'); +} + +describe('Suggestions', () => { + + it('renders a list properly', () => { + + const component = renderer.create( + + ); + const tree = component.toJSON(); + expect(tree).toMatchSnapshot(); + }); + + it('scrolls to selected index properly', () => { + const component = renderer.create( + + ); + + component.update( + + ); + const tree = component.toJSON(); + expect(tree).toMatchSnapshot(); + }); + +}); \ No newline at end of file diff --git a/packages/office-ui-fabric-react/src/components/pickers/Suggestions/__snapshots__/Suggestions.test.tsx.snap b/packages/office-ui-fabric-react/src/components/pickers/Suggestions/__snapshots__/Suggestions.test.tsx.snap new file mode 100644 index 00000000000000..69268f1244898c --- /dev/null +++ b/packages/office-ui-fabric-react/src/components/pickers/Suggestions/__snapshots__/Suggestions.test.tsx.snap @@ -0,0 +1,3095 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Suggestions renders a list properly 1`] = ` +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+
+`; + +exports[`Suggestions scrolls to selected index properly 1`] = ` +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+ +
+
+
+
+
+`; From 2e423b60dc3cf77d1d7172991ad2e5d9fbb9b79c Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Thu, 29 Mar 2018 16:30:15 -0700 Subject: [PATCH 04/10] adding change file --- ...hect-fix-pickers-suggestions_2018-03-29-23-30.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/office-ui-fabric-react/joschect-fix-pickers-suggestions_2018-03-29-23-30.json diff --git a/common/changes/office-ui-fabric-react/joschect-fix-pickers-suggestions_2018-03-29-23-30.json b/common/changes/office-ui-fabric-react/joschect-fix-pickers-suggestions_2018-03-29-23-30.json new file mode 100644 index 00000000000000..5008d0b20d6b55 --- /dev/null +++ b/common/changes/office-ui-fabric-react/joschect-fix-pickers-suggestions_2018-03-29-23-30.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "Pickers: fix bug where suggestions wouldn't have correct value selected", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "joschect@microsoft.com" +} \ No newline at end of file From af252a4e06cfb353678b3d867f57289dc7de0d38 Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Thu, 29 Mar 2018 16:34:40 -0700 Subject: [PATCH 05/10] improve test --- apps/vr-tests/.storybook/config.js | 17 +++++++++++++++++ .../src/stories/PeoplePicker.stories.tsx | 5 ++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/apps/vr-tests/.storybook/config.js b/apps/vr-tests/.storybook/config.js index 73f00e5d992c55..048b4456bff40d 100644 --- a/apps/vr-tests/.storybook/config.js +++ b/apps/vr-tests/.storybook/config.js @@ -1,3 +1,15 @@ + +if (typeof window === 'object' && window.navigator && (/node\.js/i).test(window.navigator.userAgent)) { + let addons = require('@storybook/addons').default; + let Channel = require('@storybook/channels').default; + addons.setChannel(new Channel({ + transport: { + setHandler: function() {}, + send: function() {} + } + })); +} + /*! Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. */ import * as storybook from '@storybook/react'; import { setOptions } from '@storybook/addon-options'; @@ -14,3 +26,8 @@ function loadStories() { storybook.configure(loadStories, module); + + +if (typeof window === 'object') { + window.__screener_storybook__ = require('@storybook/react').getStorybook(); +} diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index cca39588249d15..8c2b7c1300270d 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -149,9 +149,12 @@ storiesOf('PeoplePicker', module) )).add('Normal with text', () => ( Date: Thu, 29 Mar 2018 16:45:54 -0700 Subject: [PATCH 06/10] fix tests and config --- apps/vr-tests/.storybook/config.js | 17 ----------------- .../src/stories/PeoplePicker.stories.tsx | 5 +---- 2 files changed, 1 insertion(+), 21 deletions(-) diff --git a/apps/vr-tests/.storybook/config.js b/apps/vr-tests/.storybook/config.js index 048b4456bff40d..73f00e5d992c55 100644 --- a/apps/vr-tests/.storybook/config.js +++ b/apps/vr-tests/.storybook/config.js @@ -1,15 +1,3 @@ - -if (typeof window === 'object' && window.navigator && (/node\.js/i).test(window.navigator.userAgent)) { - let addons = require('@storybook/addons').default; - let Channel = require('@storybook/channels').default; - addons.setChannel(new Channel({ - transport: { - setHandler: function() {}, - send: function() {} - } - })); -} - /*! Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. */ import * as storybook from '@storybook/react'; import { setOptions } from '@storybook/addon-options'; @@ -26,8 +14,3 @@ function loadStories() { storybook.configure(loadStories, module); - - -if (typeof window === 'object') { - window.__screener_storybook__ = require('@storybook/react').getStorybook(); -} diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index 8c2b7c1300270d..a3cdf6e85a63b8 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -149,12 +149,9 @@ storiesOf('PeoplePicker', module) )).add('Normal with text', () => ( Date: Thu, 29 Mar 2018 16:55:35 -0700 Subject: [PATCH 07/10] revert test changes --- apps/vr-tests/src/stories/PeoplePicker.stories.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index a3cdf6e85a63b8..5e5c72cfbd7cc7 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -149,7 +149,10 @@ storiesOf('PeoplePicker', module) )).add('Normal with text', () => ( From 82dca88e8a100cc224a32ad5900b7437674b89e0 Mon Sep 17 00:00:00 2001 From: Jon Schectman Date: Thu, 29 Mar 2018 17:06:16 -0700 Subject: [PATCH 08/10] last test update --- apps/vr-tests/src/stories/PeoplePicker.stories.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index 5e5c72cfbd7cc7..5d8bf0c5bb1984 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -153,8 +153,6 @@ storiesOf('PeoplePicker', module) .click('.ms-BasePicker-input') .setValue('.ms-BasePicker-input', 'a') .snapshot('suggestiona') - .setValue('.ms-BasePicker-input', 'al') - .snapshot('suggestional') .end() }> Date: Thu, 29 Mar 2018 17:24:45 -0700 Subject: [PATCH 09/10] update test name and snapshot --- apps/vr-tests/src/stories/PeoplePicker.stories.tsx | 2 +- .../src/components/pickers/Suggestions/Suggestions.test.tsx | 2 +- .../Suggestions/__snapshots__/Suggestions.test.tsx.snap | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx index 5d8bf0c5bb1984..49aaefbe388bb4 100644 --- a/apps/vr-tests/src/stories/PeoplePicker.stories.tsx +++ b/apps/vr-tests/src/stories/PeoplePicker.stories.tsx @@ -152,7 +152,7 @@ storiesOf('PeoplePicker', module) .snapshot('default', { cropTo: '.testWrapper' }) .click('.ms-BasePicker-input') .setValue('.ms-BasePicker-input', 'a') - .snapshot('suggestiona') + .snapshot('suggestion: "a"') .end() }>
-
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
-
-`; - -exports[`Suggestions scrolls to selected index properly 1`] = ` -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
- -
-
-
-
-
-`;