Skip to content

Adding onPendingValueChanged callback #4105

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 17 commits into from
Mar 7, 2018
Merged

Adding onPendingValueChanged callback #4105

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ebe922e
Adding onPendingValueSet callback
madosal Feb 27, 2018
d1c5014
Adding rush change file
madosal Feb 27, 2018
51f3aa8
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
madosal Feb 27, 2018
c9626cd
Creating two props for preview and revert preview
madosal Feb 28, 2018
edb9901
Fixing some spaces
madosal Feb 28, 2018
22bf253
Changed description
manueldosal Feb 28, 2018
6c72b47
Adding currentPendingValue back to address free form
madosal Mar 1, 2018
d38795d
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
madosal Mar 1, 2018
df4f4ef
Merge branch 'ComboBox_Add_onPendingValueSet_Callback' of https://git…
madosal Mar 1, 2018
b03155c
Going back to onPendingValueChanged
madosal Mar 2, 2018
ff7d2e6
Adding some comments
madosal Mar 2, 2018
215d6f1
Renaming variable to sendUndefinedPendingValue
madosal Mar 3, 2018
33f7e26
Merge with master
madosal Mar 5, 2018
1acb059
Reducing the logic to only send one callback
madosal Mar 6, 2018
46d2fd4
Simplifying logic
madosal Mar 6, 2018
b58225b
Changing the comment description
manueldosal Mar 7, 2018
d98eb23
Fixing newPendingIndex condition
madosal Mar 7, 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 ...nges/office-ui-fabric-react/ComboBox_Add_onPendingValueSet_Callback_2018-02-27-01-11.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 adding onPendingValueSet callback prop to run when pending value is changed",
"type": "patch"
}
],
"packageName": "office-ui-fabric-react",
"email": "madosal@microsoft.com"
}
20 changes: 17 additions & 3 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 @@ -192,13 +192,17 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
allowFreeform,
value,
onMenuOpen,
onMenuDismissed
onMenuDismissed,
onPendingValueSet
} = this.props;
const {
isOpen,
focused,
currentOptions,

@manueldosal manueldosal Feb 28, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing this that I don't need it anymore

selectedIndex,
currentPendingValueValidIndex
currentPendingValue,
currentPendingValueValidIndex,
currentPendingValueValidIndexOnHover
} = this.state;

// If we are newly open or are open and the pending valid index changed,
Expand Down Expand Up @@ -243,6 +247,16 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
if (!isOpen && prevState.isOpen && onMenuDismissed) {
onMenuDismissed();
}

if (onPendingValueSet) {
if (currentPendingValue !== prevState.currentPendingValue) {
onPendingValueSet(undefined, undefined, currentPendingValue);
} else if (currentPendingValueValidIndexOnHover !== prevState.currentPendingValueValidIndexOnHover) {
onPendingValueSet(currentOptions[currentPendingValueValidIndexOnHover], currentPendingValueValidIndexOnHover, undefined);
} else if (currentPendingValueValidIndex !== prevState.currentPendingValueValidIndex) {
onPendingValueSet(currentOptions[currentPendingValueValidIndex], currentPendingValueValidIndex, undefined);

@jspurlin jspurlin Feb 27, 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.

Consider pulling this into a function to keep componentDidUpdate from getting too messy

@jspurlin jspurlin Feb 27, 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.

The else if statements are practically the same, consider simplifying to set a local variable for the index, and if that exists then call the callback using that index

}
}
}

public componentWillUnmount() {
Expand Down Expand Up @@ -347,7 +361,7 @@ export class ComboBox extends BaseComponent<IComboBoxProps, IComboBoxState> {
styles={ this._getCaretButtonStyles() }
role='presentation'
aria-hidden={ isButtonAriaHidden }
data-is-focusable={false}
data-is-focusable={ false }
tabIndex={ -1 }
onClick={ this._onComboBoxClick }
iconProps={ buttonIconProps }
Expand Down
6 changes: 6 additions & 0 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 @@ -48,6 +48,12 @@ export interface IComboBoxProps extends ISelectableDroppableTextProps<IComboBox>
*/
onChanged?: (option?: IComboBoxOption, index?: number, value?: string) => void;

/**
* Callback issued when the user changes the pending value in ComboBox
* If now value is set, we send undefined parameters
*/
onPendingValueSet?: (option?: IComboBoxOption, index?: number, value?: string) => void;

@jspurlin jspurlin Feb 27, 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.

Maybe onPendingValueChanged is better name


/**
* Function that gets invoked when the ComboBox menu is launched
*/
Expand Down
31 changes: 16 additions & 15 deletions packages/office-ui-fabric-react/src/components/ComboBox/examples/ComboBox.Basic.Example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,20 @@ export class ComboBoxBasicExample extends React.Component<{}, {
value?: string;
}> {
private _testOptions =
[{ key: 'Header', text: 'Theme Fonts', itemType: SelectableOptionMenuItemType.Header },
{ key: 'A', text: 'Arial Black' },
{ key: 'B', text: 'Times New Roman' },
{ key: 'C', text: 'Comic Sans MS' },
{ key: 'divider_2', text: '-', itemType: SelectableOptionMenuItemType.Divider },
{ key: 'Header1', text: 'Other Options', itemType: SelectableOptionMenuItemType.Header },
{ key: 'D', text: 'Option d' },
{ key: 'E', text: 'Option e' },
{ key: 'F', text: 'Option f' },
{ key: 'G', text: 'Option g' },
{ key: 'H', text: 'Option h' },
{ key: 'I', text: 'Option i' },
{ key: 'J', text: 'Option j', disabled: true },
];
[{ key: 'Header', text: 'Theme Fonts', itemType: SelectableOptionMenuItemType.Header },
{ key: 'A', text: 'Arial Black' },
{ key: 'B', text: 'Times New Roman' },
{ key: 'C', text: 'Comic Sans MS' },
{ key: 'divider_2', text: '-', itemType: SelectableOptionMenuItemType.Divider },
{ key: 'Header1', text: 'Other Options', itemType: SelectableOptionMenuItemType.Header },
{ key: 'D', text: 'Option d' },
{ key: 'E', text: 'Option e' },
{ key: 'F', text: 'Option f' },
{ key: 'G', text: 'Option g' },
{ key: 'H', text: 'Option h' },
{ key: 'I', text: 'Option i' },
{ key: 'J', text: 'Option j', disabled: true },
];

private _fontMapping: { [key: string]: string } = {
['Arial Black']: '"Arial Black", "Arial Black_MSFontService", sans-serif',
Expand Down Expand Up @@ -81,7 +81,8 @@ export class ComboBoxBasicExample extends React.Component<{}, {
onFocus={ () => console.log('onFocus called') }
onBlur={ () => console.log('onBlur called') }
onMenuOpen={ () => console.log('ComboBox menu opened') }
// tslint:enable:jsx-no-lambda
onPendingValueSet={ (option, pendingIndex, pendingValue) => console.log('Pending value was changed. Pending index: ' + pendingIndex + '. Pending value: ' + pendingValue) }
// tslint:enable:jsx-no-lambda
/>

<PrimaryButton
Expand Down