New Component: ComboBox #1920

jspurlin · 2017-06-02T18:21:18Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ npm run change

Description of changes

Add a new combobox component.

A few notes to how this differs from a dropdown:

ComboBox requires an <input> element for displaying the collapsed value to align more closely with the correct accessibility representation of a combobox
The keyboarding behavior and the ability to select available options varies greatly from dropdown
ComboBox is a "dropdown" that can be more specialize and in my mind will allow the dropdown to eventually be deprecated. Note, the combobox has the ability to autocomplete and/or allow free-form input (and the permutations of the two where autoComplete =="false" and allowFreeform=="false" aligns with the current behavior of the dropdown

The gif shows:

The first combobox has autoComplete allowsFreeform. I type "time" which autoCompetes, then I expand the menu, move around to different options and finally press ESC to cancel the expatiation of the menu discarding the pending menu option, and then arrow up and down to move to different menu options which makes the new option the active selected option
The second combobox does not autoComplete and allowsFreeform. I type "time" which does not autoComplete, then I expand the menu, continue to type one of the options in which case it gives that option the selected look, and then Press ENTER to submit the value
The third combobox has autoComplete and does not allowFreeform. I type "time" and when the combobox is in this form typing goes to a matched option and allows for typing to continue to build a pending string to allow alternate matches as long as the changes happen within one second of each other (follow the default browser behavior); the menu is then expanded and an option is selected
The fourth combobox does not autoComplete and does not allowFreeform. I type a bunch of things and get no change in options, but if I expand the menu I can choose different options
The sixth combobox is disabled and typing nor arrowing or click will cause the combobox to respond

Another side note: if allowFreeform is false, clicking on the combobox will expand the combobox as opposed to when it is true, it places an IP. Also allowFreeform allows the user to enter new options and they will get added to the options, in the uncontrolled case, or will be passed back as the value in the onChanged callback

Focus areas to test

The input and selection of options with both mouse and keyboard (via text input and arrow keys) as well as getting the option on initial render or dynamically on focus

msftclas · 2017-06-02T18:21:22Z

@jspurlin,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

jair-cazarin

Can you check the code coverage of the UTs?

I believe we should move away from inheritance, if there is some code that need to be shared then we should extract and use through composition.

It also feels like ComboBox can be broken down in smaller stateless components for better maintainability and readability.

Regarding the promise, is that a common practice in Fabric? It feels the component should just receive new props when something is done async in the app.

jair-cazarin · 2017-06-04T19:40:18Z

apps/fabric-website/src/components/App/AppState.tsx

@@ -1,398 +1,405 @@
-import * as React from 'react';


what's up with all the space changes?

I didn't make any space changes... not sure what that was

the only thing I did add is the combobox piece

jair-cazarin · 2017-06-04T19:40:51Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.Props.ts

+import { IDropdownOption, IDropdownProps } from '../../Dropdown';
+import { IIconProps } from '../../Icon';
+
+export interface IComboBox {


why do you need an empty interface?

yeah, it's not needed was just following the convention in the fabric codebase

jair-cazarin · 2017-06-04T19:45:10Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.Props.ts

+  buttonIconProps?: IIconProps;
+}
+
+export interface IComboBoxOption extends IDropdownOption {


This inheritance is fragile IMO, and some of the fields don't make sense to be inherited, DropdownMenuItemType for example.

Why not extract an ISelectableOption interface that both Dropdown and Combobox can use and have the both components decouple?

Sure, I'll rework this. It does make sense to have the contents of the DropdownMenuItemType although a better name would make it easier to think about

jair-cazarin · 2017-06-04T19:46:37Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.Props.ts

+   * Callback issued when the options should be resolved, if they have been updated or
+   * if they need to be passed in the first time
+   */
+  onResolveOptions?: () => IComboBoxOption[] | PromiseLike<IComboBoxOption[]>;


what's the difference between this function returning IComboBoxOption[] and setting options directly?

One is handling the full state outside of the component (e.g. setting the options directly) opposed to the uncontrolled case where it may take some time to get the options back but we want the state to be stored inside of the component. The promise allows for the update without blocking

jair-cazarin · 2017-06-04T19:47:45Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.Props.ts

+
+}
+
+export interface IComboBoxProps extends IDropdownProps {


can we use composition instead of inheritance?

Ideally a combo box is just a dropdown + input text so that should be reflected in the props as well, IMO

I'm more conflicted now that the DropDown component is not even used.

The comboBox doesn't use a dropdown since dropdowns allow you to style the visible collapsed where comboBoxes enforce that the rendering is inside of a <input> element which should not allow for an arbitrary render functionality

jair-cazarin · 2017-06-05T11:06:40Z

packages/office-ui-fabric-react/src/components/pickers/AutoFill/BaseAutoFill.tsx

-  private _inputElement: HTMLInputElement;
-  private _autoFillEnabled: boolean = true;
-  private _value: string;
+  protected _inputElement: HTMLInputElement;


The correct way to do this in React is through composition rather than exposing internals in a base class.

I'll look into changing this to use composition

jair-cazarin · 2017-06-05T11:17:53Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.Props.ts

+  /**
+   * Font-family associated with this option.
+   */
+  fontFamily?: string;


this is interesting, isn't this specific to our font type combo box? Isn't there already an onRenderItem or similar that would allow the consumer to specify how to render each item?

True, but I felt like this aspect is scoped enough to expose it as well without making the creator create an onRenderOption

Question for you @jair-cazarin : Do you want to handle this with onRenderOption for our use case? If so I can remove the IComboBoxOption

it feels very specific to that particular ComboBox IMO

so I'd vote to remove it

jair-cazarin · 2017-06-05T11:20:27Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+   * @param index - the index to check
+   * @returns {boolean} - true if the index is valid for the given options, false otherwise
+   */
+  @autobind


does this also need to be autobind?

nope, removed

jair-cazarin · 2017-06-05T11:23:42Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+   * allows freeform entry
+   * @param updatedValue - the input's newly changed value
+   */
+  @autobind


doesn't need autobind

can you make sure only the methods that need to be bound are decorated with autobind? there is a perf overhead otherwise.

jair-cazarin · 2017-06-05T11:37:05Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+        selectedIndex: newSelectedIndex
+      });
+
+      if (onChanged) {


should this be called after setState is done?

same response as above

micahgodbolt · 2017-06-05T15:30:05Z

Can you add a few images to the description so that we know what you're building here? @jspurlin

betrue-final-final · 2017-06-05T15:34:23Z

I would assume it's just a dropdown that you can type in? It looks like we have this in SharePoint already. @yiminwu might be able to help.

dzearing · 2017-06-06T00:46:51Z

@betrue-final-final We already have a dropdown...

@jspurlin So what is this component? Why would a customer use this vs dropdown?

dzearing

block until we get some clarification on what dropdown isn't providing.

jspurlin

Replying to the existing comments

betrue-final-final · 2017-06-06T02:44:35Z

@dzearing @jspurlin , what they are looking for is the ability to type in a value not in the dropdown. I know we do have this in SharePoint, but I don't see the ability to do this in the Fabric component.

dzearing · 2017-06-06T07:27:58Z

got it. screenshots would be handy

jspurlin · 2017-06-06T16:58:35Z

Added some more details to the description which lays out why this component ended up not either being a dropdown or being composed of a dropdown (as well as a gif showing off the functionality in the different states). My intention was to add those details earlier, but I was getting pulled other directions.

Take a look and I'd like to get your input after the update.

Note that the gif show performing similar actions across the different permutations of autoComplete and allowFreeform.

…ecoupled the shared props/memebers from comboBox and Dropdown, and extended BaseAutoFill which allowed DynamicAutoFill to be removed

…rops

…c-react into jspurlin/ComboBox

…nderOption instead. The fontFamily aspect was too specific for the generic comboBox

jair-cazarin · 2017-06-08T20:49:14Z

common/config/rush/npm-shrinkwrap.json

    "@microsoft/web-library-build": {
      "version": "3.0.2",
      "resolved": "https://registry.npmjs.org/@microsoft/web-library-build/-/web-library-build-3.0.2.tgz",
      "integrity": "sha1-DldYaol/ddjdDAKdKVtBapg4f5w=",


Do you actually need to change the ShrinkWrap file?

jair-cazarin · 2017-06-08T20:57:21Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+      buttonIconProps
+    } = this.props;
+    let { isOpen, selectedIndex, focused, suggestedDisplayValue, currentOptions } = this.state;
+    let selectedOption = currentOptions[selectedIndex];


could selectedIndex be -1 when rendering?

yes, that's a valid state and means that none of the options are selected

jair-cazarin · 2017-06-08T20:58:17Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+    return (
+      <div ref='root' className={ css('ms-ComboBox-container') }>
+        { label && (
+          <Label className={ css('ms-ComboBox-label') } id={ id + '-label' } ref={ this._resolveRef('_comboBoxLabel') } required={ required } htmlFor={ id }>{ label }</Label>


why do you need to capture the reference of the Label?

we don't, fixed

jair-cazarin · 2017-06-08T21:41:51Z

packages/office-ui-fabric-react/src/components/pickers/AutoFill/BaseAutoFill.Props.ts

+   * @param {IBaseAutoFillProps} nextProps - the props that got passed in to the auto fill's componentWillReceiveProps
+   * @returns {string} - the updated value to set, if needed
+   */
+  onComponentWillReceiveProps?: (nextProps: IBaseAutoFillProps) => string;


Typically lifecycle methods like this are used to update the component state in case of new props so exposing a callback with this name is quite weird. Can you actually rename these handlers to convey what they actually do?

and perhaps instead of exposing the entire set of props only the ones the ones you care about? It should be ideally only the ones that are in the local state (other props were set by the parent anyhow).

You're right, we already have the props and we don't need to pass them back... removed the parameter and renamed the callbacks

jair-cazarin · 2017-06-08T21:42:14Z

packages/office-ui-fabric-react/src/components/pickers/AutoFill/BaseAutoFill.tsx

-  private _inputElement: HTMLInputElement;
-  private _autoFillEnabled: boolean = true;
-  private _value: string;
+  protected _inputElement: HTMLInputElement;


these don't have to be protected anymore right?

jair-cazarin · 2017-06-09T15:13:55Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+    if (this.props.autoComplete) {
+
+      // If autoComplete is on, attempt to find a match where the text of an option starts with the updated value
+      let items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== DropdownMenuItemType.Header && option.itemType !== DropdownMenuItemType.Divider).filter((option) => option.text.toLocaleLowerCase().indexOf(updatedValue) === 0);


do you need the two filters or can it be done with one?

jair-cazarin · 2017-06-09T15:14:56Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+    } else {
+
+      // If autoComplete is off, attempt to find a match only when the value is exactly equal to the text of an option
+      let items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== DropdownMenuItemType.Header && option.itemType !== DropdownMenuItemType.Divider).filter((option) => option.text.toLocaleLowerCase() === updatedValue);


perhas extract to a function so that it can be shared with the above?

jair-cazarin · 2017-06-09T15:18:44Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+        updatedValue = updatedValue.toLocaleLowerCase();
+
+        // If autoComplete is on, attempt to find a match where the text of an option starts with the updated value
+        let items = currentOptions.map((item, index) => { return { ...item, index }; }).filter((option) => option.itemType !== DropdownMenuItemType.Header && option.itemType !== DropdownMenuItemType.Divider).filter((option) => option.text.toLocaleLowerCase().indexOf(updatedValue) === 0);


another case where we can reuse the same function

jair-cazarin · 2017-06-09T15:20:45Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+    }
+
+    // If we get here, either autoComplete is on or we did not find a match with autoComplete on.
+    // Remeber we are not allowing freeform, so at this point, if we have a pending valid value index


typo: Remember

jair-cazarin · 2017-06-09T15:22:10Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+
+    let newIndex = index + searchDirection;
+
+    newIndex = Math.max(0, Math.min(currentOptions.length - 1, newIndex));


Can this get in an infinite loop?

No, if you reach one of the ends of the array and are still at an option which shouldn't get focus, we won't make another call. Also, if the caller didn't tell us which way to move we won't try

jair-cazarin · 2017-06-12T11:48:40Z

packages/office-ui-fabric-react/src/components/SpinButton/SpinButton.tsx

@@ -1,428 +0,0 @@
-import * as React from 'react';


mm this is weird, bad merge?

* Enable no implicit any in the utilities package (microsoft#1970) * Fix no implicit anys in utilities package. * Rush change * Reverse all shrinkwrap changes except the typings one. * Fix bundle minification to exclude debug warnings. (microsoft#1973) * Updating shrinkwrap, rush, and making minify build have production flag to remove debug code. * Updates. * removing lockfile. * dropping to 7. Moving back to npm run build. * Downgrading rush. * Applying package updates. * Website: Update dev.office.com header (microsoft#1966) * Use Fabric Core 7.1.0 for the website * Adjust position of caret's in header so that they spin around central axis * Update to latest icon font from dev.office.com for header and move outside out :global{} to fix build issue * Remove a u- prefix that was missed earlier * Update dev.office.com header with the latest navigation links * Add change file * Make sure the quote rule is enabled for tsline (microsoft#1961) * With responsive mode error (microsoft#1956) * withResponsiveMode: Adding error handling around the case where window.innerWidth throws an exception * adding change log file * Create withResponsiveMode.tsx

* First step at stepper implementation. * Add first implementation of stepper. * Add functionality to stepper * Refine the Stepper class and add tests * let's make sure to put focus back on the text field when submitting via enter * Added documentation to Stepper. * Add flexibility to current stepper implementation. * Modified example implementations. * Add aria-valuemax. * Change Stepper to SpinButton. * Add example with unit. * Implement color scheme in the ContextualMenu control to enable alternative theming. * Improvements to SpinButton. * Fix increment function calls. * Add new width optional parameter. * Add label direction. * Fix border. * Add Position enum. * `defaultValue` is now the deciding prop for using the default implementation or not. * onBlur is now onValidate. * Fix tests. * Fix warnings. * Add implementation for labelGap. * Put some polish on the styling, added some icon support, and added some more example spinButtons * Implement the bar and unit tests and component page * Add the ability for the spinButton buttons to look pressed when spinning via keyboard * Revert "Implement color scheme in the ContextualMenu control to enable alternative theming." This reverts commit 4f830cd. * Don't render an empty icon for an icon-less header menu item. * Revert "Implement Document Title Bar" * update some CSS for high contrast in ff and use css utility instead of concatenating string classnames * Fix quotation issue * Fix Spin Button properties table. * Fix Spin Button example code * Use iconProps instead of string * Extracted `spinning` out of state * Add autobind instead of manually binding private functions to this * Change `+` syntax for more explicit `Number()` * Remove unnecessary cast * Fix typos * `incrementButtonIcon` and `decrementButtonIcon` are now IIconProps * Add KeyboardSpinDirection enum * Fix test description * Fix SpinButton tests * Remove unused onChange callback from SpinButton * Revert onChange * Remove old Stepper.ts file * Use module css instead of global * Fix missing word in comment * Callback functions now allow for void return (state to be updated outside) * Use `_async` instead of window * Fix minor rendering issue with browser zoom * Rename `_spinning` to `_spinningByMouse` for clarity * Fix tests * Fix extra space before label * Remove width outside of SpinButton component and fix styling * Add more tests to SpinButton * Fix SpinButton documentation * Fix typo * Fix AppDefinition for SpinButton and Spinner * Add missing documentation to SpinButton title prop * Various SpinButton fixes * Fix SpinButton path for properties * Fix SpinButton styling issues * Remove labelGap property from SpinButton

…c-react into jspurlin/ComboBox # Conflicts: # packages/office-ui-fabric-react/src/components/Dropdown/examples/Dropdown.Basic.Example.tsx

jair-cazarin · 2017-06-15T21:08:23Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+      let newOptions: ISelectableOption[] | PromiseLike<ISelectableOption[]> =
+        this.props.onResolveOptions({ ...this.state.currentOptions });
+
+      // the options are either goingto be an array or a promise


can you remove this? I don't think it really adds anything to the code, it's pretty clear from the above it can be either type.

jair-cazarin · 2017-06-15T21:09:13Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.tsx

+  }
+
+  // Render Callout container and pass in list
+  @autobind


I don't think any of these render functions need @autoBind

Yes they do need autobind since they reference "this" which won't be the expected this. I had narrowed down the autobind usages already every one that's there now needs to be there and would throw an error otherwise

jair-cazarin · 2017-06-16T16:06:19Z

packages/office-ui-fabric-react/src/components/ComboBox/ComboBox.test.tsx

+  it('Can change items in uncontrolled case', () => {
+    let comboBoxRoot;
+
+    try {


Can we remove the try/finally's? I don't think they are needed...if the test throws...it should fail and the finally is not doing any clean up or similar.

…y in tests

…c-react into jspurlin/ComboBox

…e-ui-fabric-react into jspurlin/ComboBox

jair-cazarin · 2017-06-19T18:30:51Z

packages/office-ui-fabric-react/src/components/pickers/AutoFill/BaseAutoFill.Props.ts

+   *  in to the auto fill's componentWillReceiveProps
+   * @returns {string} - the updated value to set, if needed
+   */
+  updateValueInWillReceiceProps?: () => string;


nit: there is a typo in here. Is it common to add the lifecycle function where these prop functions are used in Fabric? It looks to me as if the name itself is leaking already implementation details of the component.

it still feels weird that we are extending BaseAutoFill this way, would composition achieve the same in a more react idiomatic way? Looking at the code, doesn't seem to be that straightforward.

Fixed the typo. The name is being explicit about where the callback is being plugged in to. This functionality was created so that I could reuse the class instead of having to have almost duplicate functionality except for the changes in these lifecycle functions

jair-cazarin · 2017-06-19T18:33:23Z

packages/office-ui-fabric-react/src/utilities/selectableOption/SelectableDroppableText.Props.ts

+  * Optional callback to access the ISelectableDroppableText interface. Use this instead of ref for accessing
+  * the public methods and properties of the component.
+  */
+  componentRef?: (component: T) => void;


this is used above like: ISelectableDroppableTextProps, so the componentReference is really of type IComboBoxProps, is that the intention?

Updated so that the componentRef is really IComboBox instead

jair-cazarin · 2017-06-19T18:34:03Z

packages/office-ui-fabric-react/src/utilities/selectableOption/SelectableDroppableText.Props.ts

+  /**
+   * Collection of options for this ISelectableDroppableText
+   */
+  options?: any;


what's the reason this became any?

To allow for the options to be generic enough so that different implementers can utilize this as they see fit

jair-cazarin · 2017-06-19T18:36:41Z

packages/office-ui-fabric-react/src/components/ComboBox/examples/ComboBox.Basic.Example.tsx

+
+  @autobind
+  private _onChanged(option: ISelectableOption, index: number, value: string) {
+    if (option != null) {


it would be great to be consistent between the usages of != and !==

yup

nitishq · 2017-07-07T19:55:11Z

So glad that this component is getting build. What's ETA on this, so that Microsoft Flow team can start utilizing it.

christiango · 2017-07-07T19:59:30Z

This was merged several weeks ago. It is already available in the latest fabric releases

stepsic-microsoft-com · 2017-07-07T20:36:27Z

Forgive my ignorance, but we use this site: https://dev.office.com/fabric#/components to understand what components are available for consumption (as well as to understand best practices, etc...). Is there any sort of gate to ensure that as new components (such as ComboBox) are added they get put on that above page so that consumers can be aware of them? Thanks!

Add ComboBox functionality

6d18606

msftclas added the cla-required label Jun 2, 2017

Merge branch 'master' into jspurlin/ComboBox

3cc47d2

mikewheaton changed the title ~~Add ComboBox functionality~~ New Component: ComboBox Jun 2, 2017

Make a fix for IE where non-allowFreeform is showing the keypresses...

ec78224

jair-cazarin reviewed Jun 5, 2017

View reviewed changes

dzearing previously requested changes Jun 6, 2017

View reviewed changes

jspurlin commented Jun 6, 2017

View reviewed changes

jspurlin added 8 commits June 8, 2017 06:30

Update the PR with code review feedback. Simplified the code a lot, d…

3399566

…ecoupled the shared props/memebers from comboBox and Dropdown, and extended BaseAutoFill which allowed DynamicAutoFill to be removed

Fixing some build warnings

e43ac7b

Update my example page to explicitly not use two mutually exclusive p…

ac35bdc

…rops

Merge branch 'master' into jspurlin/ComboBox

d4c84ec

The npm-shrinkwrap.json file automatically updated... commiting

6b2ef29

Had a bad copy paste, fixing up the incorrectly logic

aad70fb

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

1c6b74b

…c-react into jspurlin/ComboBox

Removing the fontFamily aspect of the comboBox and utilizing the onRe…

1612bcc

…nderOption instead. The fontFamily aspect was too specific for the generic comboBox

jair-cazarin reviewed Jun 9, 2017

View reviewed changes

jspurlin added 2 commits June 9, 2017 10:36

Update based on feedback

3941b20

Merge branch 'master' into jspurlin/ComboBox

d52e4e3

jair-cazarin reviewed Jun 12, 2017

View reviewed changes

christiango and others added 3 commits June 12, 2017 11:16

merge

ba63a49

dzearing and others added 11 commits June 12, 2017 16:17

Create ComboBox.Props.ts

80f901e

Create ComboBox.test.tsx

2670a5d

Create ComboBox.Basic.Example.tsx

d02daaa

Create Dropdown.Props.ts

398bdd1

Fix the case sensitivity issue

12b7b7b

one more casing issue

3536017

changing the reference of utilities in the test file

55e2ad7

Actually it look like it has to be pascalCase here

c6b98ed

... really... what's going on with the casing here

460f3de

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

5bd6c64

…c-react into jspurlin/ComboBox # Conflicts: # packages/office-ui-fabric-react/src/components/Dropdown/examples/Dropdown.Basic.Example.tsx

Address feedback from in person review with David

3022036

jair-cazarin reviewed Jun 15, 2017

View reviewed changes

jair-cazarin reviewed Jun 16, 2017

View reviewed changes

jspurlin added 4 commits June 19, 2017 11:20

A few minor updated to remove uneeded comment and unneeded try/finall…

358d025

…y in tests

Merge branch 'master' into jspurlin/ComboBox

705a852

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

2634c1b

…c-react into jspurlin/ComboBox

Merge branch 'jspurlin/ComboBox' of https://github.com/jspurlin/offic…

6b0e678

…e-ui-fabric-react into jspurlin/ComboBox

jair-cazarin reviewed Jun 19, 2017

View reviewed changes

jair-cazarin approved these changes Jun 19, 2017

View reviewed changes

Fix up a typo and fix up to use consistent syntax on a few lines

9039ab4

christiango merged commit f2fc34e into microsoft:master Jun 19, 2017

microsoft locked as resolved and limited conversation to collaborators Aug 31, 2019


		let newIndex = index + searchDirection;

		newIndex = Math.max(0, Math.min(currentOptions.length - 1, newIndex));

New Component: ComboBox #1920

New Component: ComboBox #1920

Uh oh!

Conversation

jspurlin commented Jun 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request checklist

Description of changes

Focus areas to test

Uh oh!

msftclas commented Jun 2, 2017

Uh oh!

jair-cazarin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

micahgodbolt commented Jun 5, 2017

Uh oh!

betrue-final-final commented Jun 5, 2017

Uh oh!

dzearing commented Jun 6, 2017

Uh oh!

dzearing left a comment

Choose a reason for hiding this comment

Uh oh!

jspurlin left a comment

Choose a reason for hiding this comment

jspurlin commented Jun 2, 2017 •

edited

Loading

jspurlin commented Jun 6, 2017 •

edited

Loading