fix: Coalesce UI filtering menus #4972
Conversation
Signed-off-by: Simon Behar <simbeh7@gmail.com>
|
Could I suggest something a bit bigger than the drop-down - you have to scroll the drop-down which discourages usage. What about a bespoke floating panel, like this:
|
|
Tippy might work for this. |
|
Yep, was chatting with Remington about some ideas and we considered this as well |
|
Current view
|
Sorry, don't think I know what you mean. Could you clarify please? |
Single responsibility principle. Rather than write a component that must do both jobs - pop-over + drop-down - write one component for the pop-over and another for the drop-down. |
Understood, thanks! |
There was a problem hiding this comment.
Hey @alexec, I took another look at the code after reading your message on Slack and I do see that the "filtering" and the "dropdown" aspects of the code are separated (see comments). Let me know what you think and if you think I'm still misunderstanding the change you want.
| className={classNames('argo-dropdown__content', {'opened': this.state.opened, 'is-menu': this.props.isMenu})} | ||
| style={{top: this.state.top, left: this.state.left}} | ||
| ref={el => (this.content = el)}> | ||
| {children} |
There was a problem hiding this comment.
This is where we display the "filter" part of the code. This function does not make any assumptions about the content of children (which comes from props.children): The code in this function is only concerned with displaying the drop down menu and has no code regarding the actual list to be filtered.
| <ul> | ||
| {props.sections | ||
| .filter(item => item.values) | ||
| .map((item, i) => ( | ||
| <div key={i}> | ||
| <li className={classNames('top-bar__filter-item', {title: true})}> | ||
| <span>{item.title}</span> | ||
| </li> | ||
| {Object.entries(item.values) | ||
| .sort() | ||
| .map(([label, checked]) => ( | ||
| <li className={classNames('top-bar__filter-item')}> | ||
| <React.Fragment> | ||
| <Checkbox id={`filter__${label}`} checked={checked} onChange={v => item.onChange(label, v)} /> | ||
| <label htmlFor={`filter__${label}`}>{label}</label> | ||
| </React.Fragment> | ||
| </li> | ||
| ))} | ||
| </div> | ||
| ))} | ||
| </ul> |
There was a problem hiding this comment.
This is where we display the "filtering" aspect. It is passed as an argument (through props.children) to DropDown. None of the code here (even the code that deals with onChange) is known/used/needed by DropDown, it is simply passed through and displayed.
| onChange: (label: string, checked: boolean) => void; | ||
| } | ||
|
|
||
| export const FilterDropDown = (props: FilterDropDownProps) => { |
There was a problem hiding this comment.
This component is called FilterDropDown, but since it uses the abstract DropDown and adds filtering to it as content, I think it's a fair name.
Fixes #4969
WIP. Menus are now coalesced but no good unified UX yet, testing different options
Signed-off-by: Simon Behar simbeh7@gmail.com
Checklist: