Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "Add shouldInputLoseFocusOnArrowKey callback for scenario to determine if input should loose focus when arrow key is present when Tabbing is enabled on All elements or Input elements",
Copy link
Copy Markdown
Member

@dzearing dzearing Mar 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a consumer I can't make sense of the reasoning here. Can you fix some of the language? This comment goes into release notes (and it can be markdown, so use backticks around property names.)

FocusZone: adding shouldInputLoseFocusOnArrowKey optional callback for scenarios where the user presses an arrow key on an input element, but we would want to disable moving focus away.

That's just an idea but maybe you could help refine the language.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good to know. Thanks David. I have updated the change log.

"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "sayalis@microsoft.com"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "FocusZone: adding `shouldInputLoseFocusOnArrowKey` optional callback for scenarios where user press an arrow key on an input element and want it to loose focus when FocusZone disables moving focus away in case when FocusZoneTabbableElements is set to All or none",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "sayalis@microsoft.com"
}
Original file line number Diff line number Diff line change
Expand Up @@ -1259,4 +1259,49 @@ describe('FocusZone', () => {
expect(inputA.tabIndex).toBe(-1);
expect(buttonB.tabIndex).toBe(0);
});
});

it('focus should leave input box when arrow keys are pressed when tabbing is supported but shouldInputLoseFocusOnArrowKey callback method return true', () => {
const tabDownListener = jest.fn();
const component = ReactTestUtils.renderIntoDocument(
<div { ...{ onFocusCapture: _onFocus, onKeyDown: tabDownListener } }>
<FocusZone { ...{ handleTabKey: FocusZoneTabbableElements.all, isCircularNavigation: false, shouldInputLoseFocusOnArrowKey: (element) => { return true; } } }>
<input type='text' className='a' />
<button className='b'>b</button>
</FocusZone>
</div >
);

const focusZone = ReactDOM.findDOMNode(component as React.ReactInstance).firstChild as Element;

const inputA = focusZone.querySelector('.a') as HTMLElement;
const buttonB = focusZone.querySelector('.b') as HTMLElement;

setupElement(inputA, {
clientRect: {
top: 0,
bottom: 20,
left: 20,
right: 40
}
});

setupElement(buttonB, {
clientRect: {
top: 0,
bottom: 20,
left: 20,
right: 40
}
});

// InputA should be focused.
inputA.focus();
expect(lastFocusedElement).toBe(inputA);

// Pressing arrow down, input should loose the focus and the button should get the focus
ReactTestUtils.Simulate.keyDown(focusZone, { which: KeyCodes.down });
expect(lastFocusedElement).toBe(buttonB);
expect(inputA.tabIndex).toBe(-1);
expect(buttonB.tabIndex).toBe(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -804,12 +804,17 @@ export class FocusZone extends BaseComponent<IFocusZoneProps, {}> implements IFo
// We shouldn't lose focus in the following cases:
// 1. There is range selected.
// 2. When selection start is larger than 0 and it is backward.
// 3. when selection start is not the end of lenght and it is forward.
// 3. when selection start is not the end of length and it is forward.
// 4. We press any of the arrow keys when our handleTabKey isn't none or undefined (only losing focus if we hit tab)
if (isRangeSelected ||
// and if shouldInputLoseFocusOnArrowKey is defined, if scenario prefers to not loose the focus which is determined by calling the
// callback shouldInputLoseFocusOnArrowKey
if (
isRangeSelected ||
(selectionStart > 0 && !isForward) ||
(selectionStart !== inputValue.length && isForward) ||
!!this.props.handleTabKey) {
(!!this.props.handleTabKey && !(this.props.shouldInputLoseFocusOnArrowKey
&& this.props.shouldInputLoseFocusOnArrowKey(element)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to add a unit test to cover this scenario so we don't break it in the future?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(it's super easy, go to the packages/oufr folder, run npm run start-test, edit FocusZone.test.tsx and add a test.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get below error when I run start-test

image

Also what do you mean by packages/oufr folder

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind Joe told me what oufr means :D

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test

) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,13 @@ export interface IFocusZoneProps extends React.HTMLAttributes<HTMLElement | Focu
*/
handleTabKey?: FocusZoneTabbableElements;

/**
* A callback method to determine if the input element should lose focus on arrow keys
* @param {HTMLInputElement} inputElement The input element which is to loose focus.
* @returns True if input element should loose focus or false otherwise.
*/
shouldInputLoseFocusOnArrowKey?: (inputElement: HTMLInputElement) => boolean;

/**
* Whether the to check for data-no-horizontal-wrap or data-no-vertical-wrap attributes
* when determining how to move focus
Expand Down
4 changes: 2 additions & 2 deletions scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"bundlesize": [
{
"path": "../apps/test-bundle-button/dist/main.min.js",
"maxSize": "45.0 kB"
"maxSize": "45.5 kB"
}
]
}
}