Skip to content
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.

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

Tag Use Case #8

Merged
merged 30 commits into from
Jan 22, 2022
Merged

Tag Use Case #8

merged 30 commits into from
Jan 22, 2022

Conversation

krfong916
Copy link
Owner

  • add debounce

Comment on lines +10 to +13
export interface DebouncedReturnFunction<
T extends (...args: any[]) => ReturnType<T>
> {
(...args: Parameters<T>): ReturnType<T> | void;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Debounce requires a function as argument, we infer the type of function using a Generic argument. We also infer the return type

Copy link
Owner Author

Choose a reason for hiding this comment

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

Currently, we don't return anything when we call our fn, but later we'll need the inferred ReturnType

Copy link
Owner Author

@krfong916 krfong916 left a comment

Choose a reason for hiding this comment

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

We have a base implementation of useDebounce.
It satisfies our current use case, but later, we'll need to write in flush and cancel methods.

Comment on lines +29 to +33
const fnRef = useRef(fn);
const delayRef = useRef(delay);
const timerIDRef = useRef<NodeJS.Timeout | null>(null);
const isLeading = useRef(leading);
const isTrailing = useRef(trailing);
Copy link
Owner Author

Choose a reason for hiding this comment

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

We have manual control of when these values change. They are declared once, during initial render, and are not re-created each render

/**
* Re-computes the memoized value when one of the dependencies have changed
*/
const debounce = React.useMemo(() => {
Copy link
Owner Author

Choose a reason for hiding this comment

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

React.useMemo will only execute when one of the dependencies change - the function will be stored in the debounced variable.

Comment on lines +10 to +13
export interface DebouncedReturnFunction<
T extends (...args: any[]) => ReturnType<T>
> {
(...args: Parameters<T>): ReturnType<T> | void;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Currently, we don't return anything when we call our fn, but later we'll need the inferred ReturnType

Comment on lines 107 to 124
// case BL.ComboboxActions.INPUT_KEYDOWN_DELETE: {
// // does nothing, does not move the input cursor, only when the highlightedIndex is -1
// // combobox can be open
// const newState = { ...state };
// return newState;
// }
// case BL.ComboboxActions.FUNCTION_OPEN_POPUP: {
// return state;
// }
// case BL.ComboboxActions.FUNCTION_CLOSE_POPUP: {
// return state;
// }
// case BL.ComboboxActions.FUNCTION_SET_HIGHLIGHTED_INDEX: {
// return state;
// }
// case BL.ComboboxActions.FUNCTION_SELECT_ITEM: {
// return state;
// }
Copy link
Owner Author

Choose a reason for hiding this comment

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

We don't need these state change types for now, no use case as of now. Later, I could see a reality where a user (us or others) may need to make state recommendations within the onStateChange callback ... like they don't want to use a fully-loaded stateReducer as a prop, and instead, just want to handle a single piece of state

Comment on lines 19 to 22
onSelectedItemChange?: (changes: Partial<ComboboxState<Item>>) => void;
onInputValueChange?: (changes: Partial<ComboboxState<string>>) => void;
onHighightedIndexChange?: (changes: Partial<ComboboxState<number>>) => void;
onIsOpenChange?: (changes: Partial<ComboboxState<boolean>>) => void;
Copy link
Owner Author

Choose a reason for hiding this comment

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

We only pass a piece of state that changed, not the entire object (as of now), addresses #7

Comment on lines 32 to 36
// Combobox Items
export interface Item {
name: string;
contents: any;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

The user provides the type of Item. Constricting to make a user conform their data into our convention.

Copy link
Owner Author

@krfong916 krfong916 left a comment

Choose a reason for hiding this comment

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

Base version of tag use case. Will need to remove comments and implement tests, not ready for merge until complete

type: ComboBoxStateChangeTypes;
getItemFromIndex?: (index: number) => Item;
getItemFromIndex?: (index: number) => any;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Should be item, woops.

}

export interface ComboboxInputProps {
controlDispatch?: (...args: any[]) => any;
Copy link
Owner Author

Choose a reason for hiding this comment

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

Needed if the user wants to debounce the combobox input. Sure, there's many ways to skin cat, I decided to put here

const keyEvt = normalizeKey(e);
if (keyEvt.name in inputKeyDownHandlers) {
inputKeyDownHandlers[keyEvt.name]();
}
};

const inputBlurHandler = (e: React.FocusEvent<HTMLInputElement>) => {
e.preventDefault();
Copy link
Owner Author

Choose a reason for hiding this comment

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

Remove

Comment on lines 236 to 249
if (props?.controlDispatch) {
const fn = () => {
dispatch({
type: BL.ComboboxActions.INPUT_VALUE_CHANGE,
text: val
});
};
props.controlDispatch(fn);
} else {
dispatch({
type: BL.ComboboxActions.INPUT_VALUE_CHANGE,
text: e.currentTarget.value
});
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

If the user wants to control how/when the action is dispatched

Comment on lines 172 to 174
const stateChangeCallback = `on${pieceOfState}Change`;
if (stateChangeCallback in props) {
props[stateChangeCallback]({ changes: newState });
props[stateChangeCallback](newState[pieceOfState]);
Copy link
Owner Author

Choose a reason for hiding this comment

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

Addresses #7 just pass the piece of state that changed for the onStateChange callback, nothing more

Comment on lines 46 to 50
const debounce = useDebouncedCallback(
(dispatch) => {
console.log('dispatch:', dispatch);
dispatch();
},
Copy link
Owner Author

Choose a reason for hiding this comment

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

Notice, we receive control of the dispatch function as argument - we control when to fire the dispatch function (based on the timer). Bless the power of useDebounceCallback 🙏🏼

…avigates combobox grid, maintain focus on input when combobox is open and styling when result is focused
… catch this because we started with the initial state as open each time

* input cursor moves when the user navigates in the combobox grid - that should not happen
- event.preventDefault() - the default behavior is move the caret, but we manually control this

* maintain focus on the input when the combobox is open
- use state and onchange callbacks

* fix the styling of a result when focused, it shouldn't "popup" like it does (figure out why it does that, what syles make it that way)
- css classnames weren't applied, we set focus via classname and not a pseudo selector property

* abort network requests if the input is blurred
- define abortcontroller and abort the network request
- define a forceAbort function as part of the useAbort hook

* fix, when there is valid input the grid is open and shows results, when the user changes their input and the input is invalid, the popup is still open (instead of showing no results found) and has the same fixed size
- define no results found. Use stateReducer to make our own changes, based on internal state changes

* don't let the tag squish - resize. It's minimum width must fit on a single line
- use alignment and add more space for media query

* test accessibility when the grid is open
- use voiceover
…rops, fix focus on selected tags using useEffect and index
…, compose multiple prop getters for complex components
Comment on lines 172 to 173
const statePiece = capitalizeString(pieceOfState as string);
const stateChangeCallback = `on${statePiece}Change`;
Copy link
Owner Author

Choose a reason for hiding this comment

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

we are calling the piece of state that changed based on the recommendations from the reducer

Comment on lines 90 to 97
const debounce = useDebouncedCallback(
(dispatch) => {
console.log('dispatch:', dispatch);
dispatch();
},
2000,
{ trailing: true }
);
Copy link
Owner Author

Choose a reason for hiding this comment

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

we define a function callback for useDebounce.
useDebounce returns a callback. We pass the dispatch function as an argument for debounce(dispatch)

Comment on lines 139 to 140
ArrowRight: (e: React.KeyboardEvent) => {
e.preventDefault();
Copy link
Owner Author

Choose a reason for hiding this comment

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

when the input is focused, the user will sometimes navigate the grid cell using the arrow keys. According to the WAI-ARIA specification for a combobox, we need to maintain focus on the input. Though not strictly necessary, it is a good user experience if the caret position of the cursor does not change while we navigate - thus, we prevent the default behavior of the arrow keys when the user navigates in the combobox.

dispatch({
type: BL.ComboboxActions.INPUT_KEYDOWN_ARROW_RIGHT,
getItemFromIndex
});
},
ArrowLeft: () => {
ArrowLeft: (e: React.KeyboardEvent) => {
e.preventDefault();
Copy link
Owner Author

Choose a reason for hiding this comment

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

when the input is focused, the user will sometimes navigate the grid cell using the arrow keys. According to the WAI-ARIA specification for a combobox, we need to maintain focus on the input. Though not strictly necessary, it is a good user experience if the caret position of the cursor does not change while we navigate - thus, we prevent the default behavior of the arrow keys when the user navigates in the combobox.

dispatch({
type: BL.ComboboxActions.INPUT_KEYDOWN_ARROW_LEFT,
getItemFromIndex
});
},
ArrowDown: () => {
ArrowDown: (e: React.KeyboardEvent) => {
e.preventDefault();
Copy link
Owner Author

Choose a reason for hiding this comment

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

when the input is focused, the user will sometimes navigate the grid cell using the arrow keys. According to the WAI-ARIA specification for a combobox, we need to maintain focus on the input. Though not strictly necessary, it is a good user experience if the caret position of the cursor does not change while we navigate - thus, we prevent the default behavior of the arrow keys when the user navigates in the combobox.

dispatch({
type: BL.ComboboxActions.INPUT_KEYDOWN_ARROW_DOWN,
getItemFromIndex
});
},
ArrowUp: () => {
ArrowUp: (e: React.KeyboardEvent) => {
e.preventDefault();
Copy link
Owner Author

Choose a reason for hiding this comment

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

when the input is focused, the user will sometimes navigate the grid cell using the arrow keys. According to the WAI-ARIA specification for a combobox, we need to maintain focus on the input. Though not strictly necessary, it is a good user experience if the caret position of the cursor does not change while we navigate - thus, we prevent the default behavior of the arrow keys when the user navigates in the combobox.

}
const { status, data, error } = state;

const run = React.useCallback((promise) => {
Copy link
Owner Author

Choose a reason for hiding this comment

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

The run function is invoked only when we call run

Copy link
Owner Author

Choose a reason for hiding this comment

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

It also makes the async api cleaner

if (!input || input === '') return;
run(fetchTags(input));
if (!input || (prevInput.current === '' && input === '')) return;
run(fetchTags(input, getSignal));
Copy link
Owner Author

Choose a reason for hiding this comment

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

We must explicitly pass the abortSignal function in order to abort a fetch call. getSignal() is simply a getter

@@ -0,0 +1,45 @@
import React from 'react';
export function useAbortController() {
const abortControllerRef = React.useRef<AbortController>();
Copy link
Owner Author

Choose a reason for hiding this comment

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

Create an abort controller that we control when it changes/updates between renders

Comment on lines +13 to +16
// callback ran when we need to create a new abort controller
const createNewAbortController = React.useCallback(() => {
abortControllerRef.current = new AbortController();
}, []);
Copy link
Owner Author

Choose a reason for hiding this comment

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

We might cancel a request. When that happens, and a new request comes around, we need to create a new 'instance' of an abort controller

Comment on lines +18 to +23
const forceAbort = React.useCallback(() => {
if (getAbortController()) {
// abort the previous request
getAbortController().abort();
}
}, []);
Copy link
Owner Author

Choose a reason for hiding this comment

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

A convenience function that we expose as a part of our API for the user to explicitly cancel an inflight request

@@ -92,7 +93,7 @@ export function useCombobox<Item>(props: BL.ComboboxProps<Item> = {}) {
*
*/
React.useEffect(() => {
if (inputRef.current && isOpen) {
if (inputRef.current && (isOpen || props.initialIsOpen)) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

we only want to focus the input when the popup is open OR the user wants the popup to always be open

Comment on lines 2 to 10
export function mergeRefs(...refs: React.MutableRefObject<any>[]) {
return function(node: React.ReactElement) {
// iterate over every ref
// assign the node to the current ref
refs.forEach((ref) => {
ref.current = node;
});
};
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

We close over the refs passed as arguments, when React finishes its rendering phase, it looks to assign the ref a value, we iterate over all refs all assign each ref the node value.
This function is important because it allows us to compose multiple components together.
For instance the combox and multiple selection and component that implements both hooks needs a ref to the textbox. This function allows us to compose all hooks/components together.

Comment on lines +12 to +20
export function callAllEventHandlers(...fns: ((...args: any[]) => any)[]) {
return function(...args: any[]) {
fns.forEach((fn) => {
if (typeof fn === 'function') {
fn(...args);
}
});
};
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

for every fn, call the function with args. Used for composing event handlers, both internal and component usage handlers

Comment on lines 15 to 36
export function getInitialValue<Item>(
props: MultipleSelectionProps<Item>,
propKey: keyof MultipleSelectionState<Item>
): Partial<MultipleSelectionState<Item>> {
if (propKey in props) {
return props[propKey as keyof MultipleSelectionProps<Item>] as Partial<
MultipleSelectionState<Item>
>;
}

// get the user-provided initial prop state, it is a piece of state
const initialPropKey = `initial${capitalizeString(
propKey
)}` as keyof MultipleSelectionProps<Item>;
if (initialPropKey in props) {
// console.log('initialPropKey', initialPropKey);
// console.log('initialPropKey', props[initialPropKey]);
return props[initialPropKey] as Partial<MultipleSelectionState<Item>>;
}

// return values from statically defined initial state object
return initialState[propKey] as Partial<MultipleSelectionState<Item>>;
Copy link
Owner Author

Choose a reason for hiding this comment

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

The component accepts from initial props. Get those initial values if they exist

Comment on lines 153 to 204
export function useMouseAndTracker(
isOpen: boolean,
refs: React.MutableRefObject[],
handleBlur: (...args: any) => any
) {
const mouseAndTrackerRef = React.useRef({
isMouseDown: false,
isTouchMove: false
});

React.useEffect(() => {
const onMouseDown = () => {
mouseAndTrackerRef.current.isMouseDown = true;
};
const onMouseUp = (e: React.SyntheticEvent) => {
mouseAndTrackerRef.current.isMouseDown = false;
if (isOpen && !isWithinBottomline(refs, e)) {
console.log('[USE_MOUSE_TRACKER_BLUR]');
handleBlur();
}
};
const onTouchStart = () => {
mouseAndTrackerRef.current.isTouchMove = true;
};

const onTouchMove = () => {
mouseAndTrackerRef.current.isTouchMove = true;
};

const onTouchEnd = (e: React.SyntheticEvent) => {
mouseAndTrackerRef.current.isTouchMove = false;
if (isOpen && !isWithinBottomline(refs, e)) {
handleBlur();
}
};

window.addEventListener('mousedown', onMouseDown);
window.addEventListener('mouseup', onMouseUp);
window.addEventListener('touchstart', onTouchStart);
window.addEventListener('touchmove', onTouchMove);
window.addEventListener('touchend', onTouchEnd);

return function() {
window.removeEventListener('mousedown', onMouseDown);
window.removeEventListener('mouseup', onMouseUp);
window.removeEventListener('touchstart', onTouchStart);
window.removeEventListener('touchmove', onTouchMove);
window.removeEventListener('touchend', onTouchEnd);
};
}, [isOpen]);
return mouseAndTrackerRef;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

We want to register mouse events that detect when we either click or touch within or away from our component. This hook is for handling internal focusing and blurring of a component

Comment on lines +206 to +217
export function isWithinBottomline(
refs: React.MutableRefObject[],
event: React.SyntheticEvent<any>
) {
return refs.some((ref) => {
if (ref.current && event.target) {
return ref.current.contains(event.target);
} else {
return false;
}
});
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This function allows us to detect whether or not the element clicked belongs to one of the nodes that our component keeps reference of. This is important because it allows us to differentiate between elements that we own and elements that we do not.

Comment on lines +129 to +136
refs.forEach((ref) => {
if (typeof ref === 'function') {
ref(node);
} else if (ref) {
ref.current = node;
}
});
};
Copy link
Owner Author

Choose a reason for hiding this comment

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

If the ref is a ref, assign it. Otherwise, the ref is another mergeRef() function call, in that case, pass the node to the function

Comment on lines 132 to 134
} else if (currentSelectedItemIndex === -1) {
return -1;
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

When we remove an item, it may be the case that the input is not focused, or a selected item is not focused. In that case, current index === -1. We don't want to 'select' the item, so return -1

Comment on lines 62 to 69
export function canNavigateToItems(): boolean {
const selection = window.getSelection();
if (selection && selection.isCollapsed && selection.anchorOffset === 0) {
return true;
} else {
return false;
}
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This function is for the textbox. The result of the function tells the caller whether or not the cursor is at the very first position of the textbox and is collapsed

Comment on lines 42 to 54
React.useEffect(() => {
if (currentSelectedItemIndex === -1 && dropdownRef.current) {
dropdownRef.current.focus();
} else if (
currentSelectedItemIndex >= 0 &&
currentSelectedItemsRef.current &&
currentSelectedItemsRef.current[currentSelectedItemIndex]
) {
console.log(state);
console.log(currentSelectedItemsRef.current);
currentSelectedItemsRef.current[currentSelectedItemIndex].focus();
}
}, [currentSelectedItemIndex, currentSelectedItem]);
Copy link
Owner Author

Choose a reason for hiding this comment

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

highlight the selected item if it's defined. The reason why the element may not exist BUT the current selected index is because clicking on an item to remove dispatches two actions - function_remove_item and item_clicked. Suppose we have a list that contains one selected item. We cannot remove the item end then focus the item - no longer exists.

Comment on lines 343 to 356
function getItemProps(index: number) {
const handleClick = () => {
console.log('[GET_ITEM_PROPS] handle click');
dispatch({
type: ComboboxActions.ITEM_CLICK,
getItemFromIndex,
index,
selectedItem: true
});
if (inputRef.current) {
console.log('[GET_ITEM_PROPS] focus input');
inputRef.current.focus();
}
};
Copy link
Owner Author

Choose a reason for hiding this comment

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

if we're clicking an item, the input will lose focus, refocus the input

Comment on lines +106 to +110
const mouseTrackerRef = useMouseAndTracker(isOpen, [inputRef, popupRef], () => {
dispatch({
type: ComboboxActions.INPUT_BLUR
});
});
Copy link
Owner Author

Choose a reason for hiding this comment

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

wrap the dispatch in a function because it will be invoked causing an infinite re-render otherwise

@krfong916 krfong916 merged commit 4629be4 into main Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant