-
Notifications
You must be signed in to change notification settings - Fork 268
feat: Add new props for search, keyboard navigation, and scrolling #589
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import { getAriaLabel } from './a11y' | |
class DropdownTreeSelect extends Component { | ||
static propTypes = { | ||
data: PropTypes.oneOfType([PropTypes.object, PropTypes.array]).isRequired, | ||
searchTerm: PropTypes.string, | ||
clearSearchOnChange: PropTypes.bool, | ||
keepTreeOnSearch: PropTypes.bool, | ||
keepChildrenOnSearch: PropTypes.bool, | ||
|
@@ -41,7 +42,9 @@ class DropdownTreeSelect extends Component { | |
onNodeToggle: PropTypes.func, | ||
onFocus: PropTypes.func, | ||
onBlur: PropTypes.func, | ||
onSearchChange: PropTypes.func, | ||
mode: PropTypes.oneOf(['multiSelect', 'simpleSelect', 'radioSelect', 'hierarchical']), | ||
pageSize: PropTypes.number, | ||
showPartiallySelected: PropTypes.bool, | ||
disabled: PropTypes.bool, | ||
readOnly: PropTypes.bool, | ||
|
@@ -50,18 +53,21 @@ class DropdownTreeSelect extends Component { | |
inlineSearchInput: PropTypes.bool, | ||
tabIndex: PropTypes.number, | ||
disablePoppingOnBackspace: PropTypes.bool, | ||
disableKeyboardNavigation: PropTypes.bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the use case for this? At a glance it seems like an a11y blocker but people don't have to use the keyboard if they don't want to so curios as to why this is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my PR description above:
I can't use arrow keys either on the search box right now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I hear you but it's not really a regular input box - hence the special treatment.
Can this handled by adding additional functionality to the keyboard navigation logic? So that we don't have to choose between having it or not at all? |
||
} | ||
|
||
static defaultProps = { | ||
onAction: () => {}, | ||
onFocus: () => {}, | ||
onBlur: () => {}, | ||
onChange: () => {}, | ||
onSearchChange: (_) => {}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an incompatibility between the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, not sure. Will have to check with CC docs to see if they support a lint rc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can ignore the warning manually in CC dashboard as a last resort |
||
texts: {}, | ||
showDropdown: 'default', | ||
inlineSearchInput: false, | ||
tabIndex: 0, | ||
disablePoppingOnBackspace: false, | ||
disableKeyboardNavigation: true, | ||
} | ||
|
||
constructor(props) { | ||
|
@@ -73,7 +79,7 @@ class DropdownTreeSelect extends Component { | |
this.clientId = props.id || clientIdGenerator.get(this) | ||
} | ||
|
||
initNewProps = ({ data, mode, showDropdown, showPartiallySelected, searchPredicate }) => { | ||
initNewProps = ({ data, searchTerm, mode, showDropdown, showPartiallySelected, searchPredicate }) => { | ||
this.treeManager = new TreeManager({ | ||
data, | ||
mode, | ||
|
@@ -86,7 +92,12 @@ class DropdownTreeSelect extends Component { | |
if (currentFocusNode) { | ||
currentFocusNode._focused = true | ||
} | ||
const searchTermChanged = searchTerm !== prevState.searchTerm | ||
if (this.searchInput && searchTermChanged) { | ||
this.searchInput.value = searchTerm | ||
} | ||
return { | ||
searchModeOn: searchTermChanged, | ||
showDropdown: /initial|always/.test(showDropdown) || prevState.showDropdown === true, | ||
...this.treeManager.getTreeAndTags(), | ||
} | ||
|
@@ -96,7 +107,10 @@ class DropdownTreeSelect extends Component { | |
resetSearchState = () => { | ||
// clear the search criteria and avoid react controlled/uncontrolled warning | ||
if (this.searchInput) { | ||
this.searchInput.value = '' | ||
if (this.searchInput.value !== '') { | ||
this.props.onSearchChange(this.searchInput.value) | ||
this.searchInput.value = '' | ||
} | ||
} | ||
|
||
return { | ||
|
@@ -154,6 +168,7 @@ class DropdownTreeSelect extends Component { | |
this.props.keepChildrenOnSearch | ||
) | ||
const searchModeOn = value.length > 0 | ||
this.props.onSearchChange(value) | ||
|
||
this.setState({ | ||
tree, | ||
|
@@ -238,6 +253,10 @@ class DropdownTreeSelect extends Component { | |
} | ||
|
||
onKeyboardKeyDown = e => { | ||
if (this.props.disableKeyboardNavigation) { | ||
return // Will fire the default action | ||
} | ||
|
||
const { readOnly, mode, disablePoppingOnBackspace } = this.props | ||
const { showDropdown, tags, searchModeOn, currentFocus } = this.state | ||
const tm = this.treeManager | ||
|
@@ -360,6 +379,7 @@ class DropdownTreeSelect extends Component { | |
onCheckboxChange={this.onCheckboxChange} | ||
onNodeToggle={this.onNodeToggle} | ||
mode={mode} | ||
pageSize={this.props.pageSize} | ||
showPartiallySelected={this.props.showPartiallySelected} | ||
{...commonProps} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,6 +10,8 @@ declare module 'react-dropdown-tree-select' { | |||||||||
|
||||||||||
export interface DropdownTreeSelectProps { | ||||||||||
data: TreeData | ||||||||||
/** Initialize the search input with the specified term and search the nodes */ | ||||||||||
searchTerm?: str | ||||||||||
/** Clear the input search if a node has been selected/unselected */ | ||||||||||
clearSearchOnChange?: boolean | ||||||||||
/** Displays search results as a tree instead of flattened results */ | ||||||||||
|
@@ -54,6 +56,9 @@ declare module 'react-dropdown-tree-select' { | |||||||||
* This is helpful for setting dirty or touched flags with forms | ||||||||||
*/ | ||||||||||
onBlur?: () => void | ||||||||||
/** Fires when search input is modified. | ||||||||||
*/ | ||||||||||
onSearchChange?: (searchTerm: str) => void | ||||||||||
/** Defines how the dropdown is rendered / behaves | ||||||||||
* | ||||||||||
* - multiSelect | ||||||||||
|
@@ -79,6 +84,8 @@ declare module 'react-dropdown-tree-select' { | |||||||||
* | ||||||||||
* */ | ||||||||||
mode?: Mode | ||||||||||
/** The size (in nodes) of a single page in the infinite scroll component. */ | ||||||||||
pageSize?: number | ||||||||||
/** If set to true, shows checkboxes in a partial state when one, but not all of their children are selected. | ||||||||||
* Allows styling of partially selected nodes as well, by using :indeterminate pseudo class. | ||||||||||
* Simply add desired styles to .node.partial .checkbox-item:indeterminate { ... } in your CSS | ||||||||||
|
@@ -103,6 +110,8 @@ declare module 'react-dropdown-tree-select' { | |||||||||
* search bar, the tree will not deselect nodes. | ||||||||||
*/ | ||||||||||
disablePoppingOnBackspace?: boolean | ||||||||||
/** dsiableKeyboardNavigation will disable keyboard navigation of the tree */ | ||||||||||
dsiableKeyboardNavigation?: boolean | ||||||||||
Comment on lines
+113
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
} | ||||||||||
|
||||||||||
export interface DropdownTreeSelectState { | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gandhis1 I did think about it during the infinite scroll implementation but ultimately decided against it. Reasons being