Skip to content

ComboBox: Add any event as additional parameter to onChanged callback for saving pending changes #4594

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
jspurlin merged 12 commits into from
Apr 19, 2018
Merged

ComboBox: Add any event as additional parameter to onChanged callback for saving pending changes #4594

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
46fdc33
added new paramater to callback
Apr 18, 2018
d853f6b
added change file
Apr 18, 2018
b3ae60d
updated the combobox type import
Apr 18, 2018
564fdad
changed callback to include all events instead of just keycode
Apr 18, 2018
d5f18be
made events more generic so user can parse it themselves
Apr 18, 2018
fc51d4b
made event optional
Apr 18, 2018
baaf37c
Merge https://github.com/OfficeDev/office-ui-fabric-react into chiech…
Apr 18, 2018
d6d02e9
re-arranged function call and added more any events
Apr 18, 2018
836b00a
removed unneeded parameter added
Apr 18, 2018
b3551c7
updated change file
Apr 19, 2018
f322efa
fixed type
Apr 19, 2018
55b0465
changed event to any to prevent crashes
Apr 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
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/chiechan-newComboboxCallback_2018-04-18-00-34.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "office-ui-fabric-react",
"comment": "ComboBox: Added KeyCode pressed as additional paramater to onChanged callback",

@jspurlin jspurlin Apr 18, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This (and the title to the PR) need to be updated

"type": "minor"
}
],
"packageName": "office-ui-fabric-react",
"email": "chiechan@microsoft.com"
}
27 changes: 14 additions & 13 deletions packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
getClassNames,
getComboBoxOptionClassNames
} from './ComboBox.classNames';
import { BaseButton, Button } from 'src/index.bundle';

export interface IComboBoxState {

Expand Down Expand Up @@ -750,7 +751,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
* @param index - the index to set (or the index to set from if a search direction is provided)
* @param searchDirection - the direction to search along the options from the given index
*/
private _setSelectedIndex(index: number, searchDirection: SearchDirection = SearchDirection.none) {
private _setSelectedIndex(index: number, searchDirection: SearchDirection = SearchDirection.none, submitPendingValueEvent?: any) {

@jspurlin jspurlin Apr 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you look at making this new parameter the second parameter? Doing that might make it to where you have to pass searchDirection in the cases where you don't need

const { onChanged, onPendingValueChanged } = this.props;
const { currentOptions } = this.state;
let { selectedIndices } = this.state;
Expand Down Expand Up @@ -798,7 +799,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {

// Did the creator give us an onChanged callback?
if (onChanged) {
onChanged(option, index);
onChanged(option, index, undefined, submitPendingValueEvent);
}

// if we have a new selected index,
Expand Down Expand Up @@ -874,15 +875,15 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
if (this.state.focused) {
this.setState({ focused: false });
if (!this.props.multiSelect) {
this._submitPendingValue();
this._submitPendingValue(event);
}
}
}

/**
* Submit a pending value if there is one
*/
private _submitPendingValue() {
private _submitPendingValue(submitPendingValueEvent: any) {
const {
onChanged,
allowFreeform,
Expand Down Expand Up @@ -914,14 +915,14 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
currentPendingValue.length + (this._comboBox.value.selectionEnd - this._comboBox.value.selectionStart) === pendingOptionText.length) ||
(this._comboBox.value && this._comboBox.value.inputElement && this._comboBox.value.inputElement.value.toLocaleLowerCase() === pendingOptionText)
)) {
this._setSelectedIndex(currentPendingValueValidIndex);
this._setSelectedIndex(currentPendingValueValidIndex, SearchDirection.none, submitPendingValueEvent);
this._clearPendingInfo();
return;
}
}

if (onChanged) {
onChanged(undefined, undefined, currentPendingValue);
onChanged(undefined, undefined, currentPendingValue, submitPendingValueEvent);
} else {
// If we are not controlled, create a new option
const newOption: IComboBoxOption = { key: currentPendingValue, text: currentPendingValue };
Expand All @@ -940,10 +941,10 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
} else if (currentPendingValueValidIndex >= 0) {
// Since we are not allowing freeform, we must have a matching
// to be able to update state
this._setSelectedIndex(currentPendingValueValidIndex);
this._setSelectedIndex(currentPendingValueValidIndex, SearchDirection.none, submitPendingValueEvent);
} else if (currentPendingValueValidIndexOnHover >= 0) {
// If all else failed and we were hovering over an item, select it
this._setSelectedIndex(currentPendingValueValidIndexOnHover);
this._setSelectedIndex(currentPendingValueValidIndexOnHover, SearchDirection.none, submitPendingValueEvent);
}

// Finally, clear the pending info
Expand Down Expand Up @@ -1241,9 +1242,9 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
* to select the item and also close the menu
* @param index - the index of the item that was clicked
*/
private _onItemClick(index: number | undefined): () => void {
return (): void => {
this._setSelectedIndex(index as number);
private _onItemClick(index: number | undefined): (ev: any) => void {
return (ev: React.MouseEvent<any> | React.FormEvent<any>): void => {

@jspurlin jspurlin Apr 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can be updated to any

this._setSelectedIndex(index as number, undefined, ev);

@jspurlin jspurlin Apr 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be searchDirection.none instead of undefined?

if (!this.props.multiSelect) {
// only close the callout when it's in single-select mode
this.setState({
Expand Down Expand Up @@ -1471,7 +1472,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {

switch (ev.which) {
case KeyCodes.enter:
this._submitPendingValue();
this._submitPendingValue(ev);
if (this.props.multiSelect && isOpen) {
this.setState({
currentPendingValueValidIndex: index
Expand Down Expand Up @@ -1499,7 +1500,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
case KeyCodes.tab:
// On enter submit the pending value
if (!this.props.multiSelect) {
this._submitPendingValue();
this._submitPendingValue(ev);
}

// If we are not allowing freeform
Expand Down
4 changes: 2 additions & 2 deletions packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ISelectableOption } from '../../utilities/selectableOption/SelectableOp
import { ISelectableDroppableTextProps } from '../../utilities/selectableOption/SelectableDroppableText.types';
import { IStyle, ITheme } from '../../Styling';
import { IButtonStyles } from '../../Button';
import { IRenderFunction } from '../../Utilities';
import { IRenderFunction, KeyCodes } from '../../Utilities';

@jspurlin jspurlin Apr 18, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You no longer need KeyCodes

import { IComboBoxClassNames } from './ComboBox.classNames';

export interface IComboBox {
Expand Down Expand Up @@ -53,7 +53,7 @@ export interface IComboBoxProps extends ISelectableDroppableTextProps<IComboBox>
* 2) a manually edited value is submitted. In this case there may not be a matched option if allowFreeform is also true
* (and hence only value would be true, the other parameter would be null in this case)
*/
onChanged?: (option?: IComboBoxOption, index?: number, value?: string) => void;
onChanged?: (option?: IComboBoxOption, index?: number, value?: string, submitPendingValueEvent?: any) => void;

/**
* Callback issued when the user changes the pending value in ComboBox
Expand Down