Skip to content

Have optional callback in focusZoneProps to determine if scenario owner wants the input element to lose the focus on arrow key #4290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dzearing merged 10 commits into from
Mar 19, 2018
Merged

Have optional callback in focusZoneProps to determine if scenario owner wants the input element to lose the focus on arrow key #4290

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b6e47f9
Test
sayali26 Mar 15, 2018
a3e9c76
Scenario determines if to loose focus or not
sayali26 Mar 15, 2018
cd8705b
update
sayali26 Mar 15, 2018
262634f
PR comment
sayali26 Mar 16, 2018
e229a77
Add returns
sayali26 Mar 16, 2018
ed17e96
Add test
sayali26 Mar 16, 2018
abd0b7a
update test
sayali26 Mar 16, 2018
c1c3da1
Update change log
sayali26 Mar 16, 2018
446d2e6
Merge remote-tracking branch 'upstream/master' into sayali/arrowFocusFix
sayali26 Mar 19, 2018
995f35b
Increase bundle maxsize
sayali26 Mar 19, 2018
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
13 changes: 10 additions & 3 deletions packages/office-ui-fabric-react/src/components/FocusZone/FocusZone.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,16 +804,23 @@ export class FocusZone extends BaseComponent<IFocusZoneProps, {}> implements IFo
const selectionEnd = element.selectionEnd;
const isRangeSelected = selectionStart !== selectionEnd;
const inputValue = element.value;
const shouldInputLoseFocusOnArrowKey = this.props.shouldInputLoseFocusOnArrowKey
? this.props.shouldInputLoseFocusOnArrowKey(element)
: false;

@micahgodbolt micahgodbolt Mar 16, 2018

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.

I don't love that the const shouldInputLoseFocusOnArrowKey is a boolean, but this.props.shouldInputLoseFocusOnArrowKey is a function returning a boolean.

Can we either move this function down to 822, or come up with a different name stating that this is the result of the function?

@sayali26 sayali26 Mar 16, 2018

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 have addressed this comment


// 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 && !shouldInputLoseFocusOnArrowKey)
) {
return false;
}
}
Expand Down
6 changes: 6 additions & 0 deletions packages/office-ui-fabric-react/src/components/FocusZone/FocusZone.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ 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.
*/

@micahgodbolt micahgodbolt Mar 16, 2018

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.

add default value down here.

@sayali26 sayali26 Mar 16, 2018

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.

not sure if we could add default value to the callback method. Added @returns

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