feat: add openOnFocus and remove minLength #31

eunjae-lee · 2020-02-24T15:51:44Z

Summary

This PR adds openOnFocus and removes minLength.

eunjae-lee · 2020-02-24T15:54:48Z

packages/autocomplete-core/src/propGetters.ts

@@ -228,7 +226,7 @@ export function getPropGetters<TItem>({
          providedProps.inputElement ===
            props.environment.document.activeElement &&
          !store.getState().isOpen &&
-          store.getState().query.length >= props.minLength
+          props.openOnFocus


This kind of sounds funny to me.

When it's not open

and it's meant to be open on focus

then let's give it a focus

(so that it will be open)

I don't have any other idea. So we can leave it as is.

I think this needs to be: (props.openOnFocus || store.getState().query.length > 0).

francoischalifour

As explained in DM, the option name that we originally though about (openOnFocus) is a bit misleading because it means "open on focus when there's no query". Either way, we always want to open the dropdown on focus.

Therefore I reviewed this PR with the intended behavior. We can rename the option later if we find a better name (openOnEmptyQuery?).

francoischalifour · 2020-02-25T09:53:36Z

packages/autocomplete-core/src/propGetters.ts

-            setContext,
-          });
-        }
+        onInput({


We need to wrap this with if (props.openOnFocus) otherwise, the dropdown opens when you click "reset" without the option openOnFocus.

francoischalifour · 2020-02-25T10:21:16Z

packages/autocomplete-core/src/onInput.ts

@@ -55,21 +55,6 @@ export function onInput<TItem>({
  setHighlightedIndex(props.defaultHighlightedIndex);
  setQuery(query);

-  if (query.length < props.minLength) {


We want to keep this branch, but the condition should be query.length === 0 && props.openOnFocus === false. This makes sure to "reset" the state.

The reasoning behind this is that as opposed to InstantSearch, when there's no query, we do not want to consider any results, except when openOnFocus is true. This behavior would be easier to validate once we write integration tests.

francoischalifour · 2020-02-25T10:25:34Z

packages/autocomplete-core/src/onInput.ts

@@ -117,7 +102,7 @@ export function onInput<TItem>({
          setSuggestions(suggestions as any);
          setIsOpen(
            nextState.isOpen ??
-              (query.length >= props.minLength &&
+              (props.openOnFocus ||


openOnFocus should always be paired with a query length check: (query.length === 0 && props.openOnFocus) || /* ... */.

francoischalifour · 2020-02-25T10:26:36Z

packages/autocomplete-core/src/propGetters.ts

-      if (store.getState().query.length >= props.minLength) {
+      // We want to trigger a query when `openOnFocus` is true
+      // because the dropdown should open with the current query.
+      if (props.openOnFocus) {


I think we want: props.openOnFocus || store.getState().query.length > 0.

francoischalifour · 2020-02-25T10:29:47Z

packages/autocomplete-core/src/propGetters.ts

@@ -228,7 +226,7 @@ export function getPropGetters<TItem>({
          providedProps.inputElement ===
            props.environment.document.activeElement &&
          !store.getState().isOpen &&
-          store.getState().query.length >= props.minLength
+          props.openOnFocus


I think this needs to be: (props.openOnFocus || store.getState().query.length > 0).

francoischalifour · 2020-02-25T10:30:34Z

packages/autocomplete-core/src/stateReducer.ts

@@ -115,7 +115,7 @@ export const stateReducer: Reducer = (action, state, props) => {
      return {
        ...state,
        highlightedIndex: props.defaultHighlightedIndex,
-        isOpen: state.query.length >= props.minLength,
+        isOpen: props.openOnFocus,


props.openOnFocus || state.query.length > 0

README.md

eunjae-lee · 2020-02-25T10:38:49Z

openOnEmptyFocus.. ?

Co-Authored-By: François Chalifour <francoischalifour@users.noreply.github.com>

…ocomplete.js into feat/open-on-focus

francoischalifour · 2020-02-25T14:21:46Z

packages/autocomplete-core/src/propGetters.ts

@@ -228,7 +227,7 @@ export function getPropGetters<TItem>({
          providedProps.inputElement ===
            props.environment.document.activeElement &&
          !store.getState().isOpen &&
-          store.getState().query.length >= props.minLength
+          (props.openOnFocus || store.getState().query.length > 0)


Actually this condition is already checked in the onFocus function. We can remove it.

82a8b5a

like this, right?

packages/autocomplete-core/src/types/api.ts

Co-Authored-By: François Chalifour <francoischalifour@users.noreply.github.com>

…ocomplete.js into feat/open-on-focus

feat: add openOnFocus and remove minLength

371f3af

eunjae-lee commented Feb 24, 2020

View reviewed changes

eunjae-lee requested a review from francoischalifour February 24, 2020 15:56

francoischalifour reviewed Feb 25, 2020

View reviewed changes

eunjae-lee and others added 5 commits February 25, 2020 14:16

fix: adjust dropdown behavior after resetting based on openOnFocus prop

9100bfe

Update README.md

390c2d8

Co-Authored-By: François Chalifour <francoischalifour@users.noreply.github.com>

fix: reset the state on empty query

4da7651

fix: update conditions to check query length

88cae65

Merge branch 'feat/open-on-focus' of github.com:francoischalifour/aut…

0dcd74b

…ocomplete.js into feat/open-on-focus

eunjae-lee requested a review from francoischalifour February 25, 2020 14:00

francoischalifour approved these changes Feb 25, 2020

View reviewed changes

eunjae-lee and others added 3 commits February 25, 2020 15:27

Update packages/autocomplete-core/src/types/api.ts

93719f1

Co-Authored-By: François Chalifour <francoischalifour@users.noreply.github.com>

fix: remove unnecessary condition

82a8b5a

Merge branch 'feat/open-on-focus' of github.com:francoischalifour/aut…

43b13fc

…ocomplete.js into feat/open-on-focus

eunjae-lee merged commit 553ea68 into next Feb 25, 2020

francoischalifour deleted the feat/open-on-focus branch February 25, 2020 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add openOnFocus and remove minLength #31

feat: add openOnFocus and remove minLength #31

eunjae-lee commented Feb 24, 2020 •

edited

Loading

eunjae-lee Feb 24, 2020 •

edited

Loading

francoischalifour Feb 25, 2020

francoischalifour left a comment

francoischalifour Feb 25, 2020

francoischalifour Feb 25, 2020

francoischalifour Feb 25, 2020

francoischalifour Feb 25, 2020

francoischalifour Feb 25, 2020

francoischalifour Feb 25, 2020

eunjae-lee commented Feb 25, 2020

francoischalifour Feb 25, 2020

eunjae-lee Feb 25, 2020

francoischalifour Feb 25, 2020

feat: add openOnFocus and remove minLength #31

feat: add openOnFocus and remove minLength #31

Conversation

eunjae-lee commented Feb 24, 2020 • edited Loading

eunjae-lee Feb 24, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

francoischalifour left a 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

eunjae-lee commented Feb 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjae-lee commented Feb 24, 2020 •

edited

Loading

eunjae-lee Feb 24, 2020 •

edited

Loading