feat: [DRAFT] adds select component #3902

eljefe223 · 2020-09-15T17:26:03Z

Description

Adds select component. Work in progress

Motivation & context

Issue type checklist

Chore: A change that does not impact distributed packages.
Bug fix: A change that fixes an issue, link to the issue above.
New feature: A change that adds functionality.

Is this a breaking change?

This change causes current functionality to break.

Adding or modifying component(s) in @microsoft/fast-components checklist

I have added a new component
I have modified an existing component
I have updated the definition file
I have updated the configuration file

Process & policy checklist

I have added tests for my changes.
I have tested my changes.
I have updated the project documentation to reflect my changes.
I have read the CONTRIBUTING documentation and followed the standards for this project.

EisenbergEffect · 2020-09-15T17:52:22Z

I'd love to see some screenshots or animated gifs of this in action 😉

scomea · 2020-09-16T18:45:00Z

packages/web-components/fast-components/docs/api-report.md

@@ -18,14 +18,18 @@ import { Dialog } from '@microsoft/fast-foundation';
 import { Direction } from '@microsoft/fast-web-utilities';
 import { Divider } from '@microsoft/fast-foundation';
 import { Flipper } from '@microsoft/fast-foundation';
+import { Listbox } from '@microsoft/fast-foundation';


I think we should have a seperate pr/spec for listbox if we're calling it the "FAST listbox" which implies it is a generic building block component.

scomea · 2020-09-16T18:50:24Z

packages/web-components/fast-foundation/src/select/select.ts

+        // Don't scroll the page for arrow keys, and spacebar.
+        e.preventDefault();
+        switch (e.keyCode) {
+            case KeyCodes.arrowDown:


Handling navigation across the options should be handled by the "listbox" component IMO.

scomea · 2020-09-16T18:56:20Z

packages/web-components/fast-foundation/src/select/select.ts

+    private typeAheadTimeoutHandler: number = -1;
+    private typeAheadExpired: boolean = false;
+    private static readonly TYPE_AHEAD_TIMEOUT_MS = 1000;
+    public handleTypeAhead(typedKey) {


Do we need to have type ahead as part of the initial feature set? Seems like an extra. There should be an option to enable/disable it at least.

I think we should look to include basic type ahead here - I don't think we need full combobox behavior.

I'm definitely in favor of this being configurable per @scomea's suggestion. I'd suggest including a property for the timeout itself so that's configurable as well. I know most browsers tend to use 500ms for this from some Open UI research we just discussed over there.

chrisdholt · 2020-09-25T00:08:57Z

packages/web-components/fast-foundation/src/select/listbox.ts

+    }
+
+    public keypressHandler = (e: KeyboardEvent): void => {
+        console.log('button pressed');


Please pull the console.logs :)

chrisdholt · 2020-09-25T00:09:02Z

packages/web-components/fast-foundation/src/select/listbox.ts

+        console.log('button pressed');
+        switch (e.keyCode) {
+            case keyCodeSpace:
+                console.log('space pressed');


Please pull the console.logs :)

What should we do in this case? Have we not yet set up our keyboarding behavior of the listbox?

chrisdholt · 2020-09-25T00:09:32Z

packages/web-components/fast-foundation/src/select/listbox.ts

+
+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
+    public clickHandler = (e: MouseEvent): void => {
+        console.log(e);


Is this a TODO?

chrisdholt · 2020-09-25T00:10:07Z

packages/web-components/fast-foundation/src/select/option.template.ts

+
+export const OptionTemplate = html<Option>`
+    <template
+        tabindex="0"


Options can be disabled - I'd expect us to have a tabindex of -1 in that case.

chrisdholt · 2020-09-25T00:10:38Z

packages/web-components/fast-foundation/src/select/option.ts

+    /* eslint-disable-next-line @typescript-eslint/no-unused-vars */
+    public clickHandler = (e: MouseEvent): void => {
+        (this.checked) ? this.checked = false : this.checked = true;
+        this.$emit("oui-option-selection-change");


we'll want this event to be different - is this a change event?

generic or "selection-change"

chrisdholt · 2020-09-25T00:11:09Z

packages/web-components/fast-foundation/src/select/select.template.ts

+
+export const SelectTemplate = html<Select>`
+    <template
+        @oui-option-selection-change="${(x, c) => {


@change - we should drop all the oui modifiers.

chrisdholt · 2020-09-25T00:11:42Z

packages/web-components/fast-foundation/src/select/select.ts

+        switch (e.keyCode) {
+            case KeyCodes.space:
+                this.open = !this.open;
+                console.log("Call one")


chrisdholt · 2020-09-25T00:11:47Z

packages/web-components/fast-foundation/src/select/select.ts

+                break;
+            case KeyCodes.enter:
+                this.open = !this.open;
+                console.log("Call two")


chrisdholt · 2020-09-25T00:11:59Z

packages/web-components/fast-foundation/src/select/select.ts

+                break;
+            case KeyCodes.enter:
+                this.value = this.options[this.activeOptionIndex].value;
+                console.log("Fire Enter", this.activeOptionIndex)


chrisdholt · 2020-09-25T00:12:04Z

packages/web-components/fast-foundation/src/select/select.ts

+            case KeyCodes.escape:
+                this.options[this.activeOptionIndex].removeAttribute('current');
+                this.open = !this.open;
+                console.log("Call three")


chrisdholt · 2020-09-25T00:12:32Z

packages/web-components/fast-foundation/src/select/select.ts

+        this.updateSelectValue(value);
+        this.typeAheadValue = '';
+        this.open = !this.open;
+        console.log("Call six")


no console please

chrisdholt · 2020-09-25T00:12:45Z

packages/web-components/fast-foundation/src/select/select.ts

+
+    private applyButtonControllerCode(): void {
+        if (this.button) {
+            console.log("This hits", this.button)


no console please

nicholasrice · 2020-09-28T16:09:12Z

Converting to draft because this is a draft PR.

stale · 2020-10-12T16:39:44Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

radium-v · 2020-10-12T17:21:35Z

stalebot pls

EisenbergEffect · 2020-10-15T13:48:53Z

@radium-v You picked up work on this, yes? Do you need further review of this PR or is it better to wait for your update?

radium-v · 2020-10-15T16:50:43Z

@EisenbergEffect I'll have more commits incoming on this soon, should I add it to this PR or create a new one?

EisenbergEffect · 2020-10-15T16:52:20Z

@radium-v I don't have any strong opinions, so feel free to do whatever works best for your flow.

chrisdholt · 2020-10-15T16:56:15Z

@radium-v I don't have any strong opinions, so feel free to do whatever works best for your flow.

@EisenbergEffect @radium-v let's snap a new PR and start fresh.

radium-v · 2020-10-26T22:04:00Z

Closing, see #4074.

eljefe223 requested a review from nicholasrice September 15, 2020 17:26

eljefe223 requested review from awentzel, chrisdholt, EisenbergEffect, Falkicon and janechu as code owners September 15, 2020 17:26

eljefe223 self-assigned this Sep 15, 2020

eljefe223 added 2 commits September 15, 2020 12:33

adds select

e1c0981

adds keycodes

9b7c356

eljefe223 force-pushed the users/jes/select-component branch from 0f93cfb to 9b7c356 Compare September 15, 2020 19:40

scomea reviewed Sep 16, 2020

View reviewed changes

eljefe223 assigned radium-v Sep 18, 2020

chrisdholt reviewed Sep 25, 2020

View reviewed changes

nicholasrice marked this pull request as draft September 28, 2020 16:09

stale bot added the warning:stale No recent activity within a reasonable amount of time label Oct 12, 2020

stale bot removed the warning:stale No recent activity within a reasonable amount of time label Oct 12, 2020

radium-v closed this Oct 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [DRAFT] adds select component #3902

feat: [DRAFT] adds select component #3902

eljefe223 commented Sep 15, 2020

EisenbergEffect commented Sep 15, 2020

scomea Sep 16, 2020

scomea Sep 16, 2020

scomea Sep 16, 2020

chrisdholt Sep 16, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

chrisdholt Sep 25, 2020

nicholasrice commented Sep 28, 2020

stale bot commented Oct 12, 2020

radium-v commented Oct 12, 2020

EisenbergEffect commented Oct 15, 2020

radium-v commented Oct 15, 2020

EisenbergEffect commented Oct 15, 2020

chrisdholt commented Oct 15, 2020

radium-v commented Oct 26, 2020

feat: [DRAFT] adds select component #3902

feat: [DRAFT] adds select component #3902

Conversation

eljefe223 commented Sep 15, 2020

Description

Motivation & context

Issue type checklist

Process & policy checklist

EisenbergEffect commented Sep 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicholasrice commented Sep 28, 2020

stale bot commented Oct 12, 2020

radium-v commented Oct 12, 2020

EisenbergEffect commented Oct 15, 2020

radium-v commented Oct 15, 2020

EisenbergEffect commented Oct 15, 2020

chrisdholt commented Oct 15, 2020

radium-v commented Oct 26, 2020