Skip to content

Initial implementation of Picker#283

Merged
dannify merged 8 commits intomasterfrom
picker
Mar 17, 2020
Merged

Initial implementation of Picker#283
dannify merged 8 commits intomasterfrom
picker

Conversation

@devongovett
Copy link
Copy Markdown
Member

This is an initial version of the Picker component, along with the useSelect aria hook and useSelectState stately hook. See individual commit messages for some details about related changes.

A few notable new features:

  • You can type to select when focus is on the field button, without opening the menu. This is similar to how a native <select> works.
  • The menu opens on mouse down rather than on mouse up, and you can drag over an item with your mouse still down and release to select that item. Also similar to how a native <select> works.

I also had to refactor ListBox into ListBoxBase so that Picker can access it directly. This is to allow the collection state and the layout to be held in the Picker rather than the ListBox. This enables type to select without opening the menu, along with layout info to be cached rather than recomputed every time the ListBox is mounted.

There is still a bunch left to do, including tests and a few more features and bug fixes. Just wanted to get an initial implementation in first and iterate on that rather than a giant PR.

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

onPressUp({
type: 'pressup',
pointerType,
target: originalEvent.target as HTMLElement,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

spread the original event?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That would include a lot more properties than we want.

...triggerAriaProps,
id: menuTriggerId,
onPress,
onPressStart: onPress,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't follow this change, can you explain?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

see PR description.


// Aria 1.1 supports multiple values for aria-haspopup other than just menus.
// https://www.w3.org/TR/wai-aria-1.1/#aria-haspopup
// However, we only add it for menus for now because screen readers often
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this comment is now untrue?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

menus and listboxes are close enough where if it is exposed as a menu it's not that big of a deal. we did this in v2.

return key;
}

return '';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

will this prevent languages like Chinese?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nope, see above comment.

private indentationForItem?: (collection: Collection<Node<T>>, key: Key) => number;
private layoutInfos: {[key: string]: LayoutInfo};
private contentHeight: number;
collection: Collection<Node<T>>;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

private?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nope, it must be set by the user


export interface PressEvent {
type: 'pressstart' | 'pressend' | 'press',
type: 'pressstart' | 'pressend' | 'pressup' | 'press',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what's the difference between pressup and pressend?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

press up is like mouse up: fired whenever the user releases a pointer over the target. press end is fired whenever the user's pointer leaves the target or is released. press is fired if there is a pointer down event followed by a pointer up event over the target.

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

@adobe-bot
Copy link
Copy Markdown

Build successful! 🎉

@dannify dannify merged commit a45db44 into master Mar 17, 2020
@dannify dannify deleted the picker branch March 17, 2020 23:13
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.

4 participants