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

Make Toolbar a controlled component so that selected filters can be managed externally #1222

Closed
danwatford opened this issue May 25, 2022 · 3 comments · Fixed by #1262
Closed
Labels
status:fixed-next-drop Issue will be fixed in upcoming release. type:enhancement New feature or enhancement of existing capability
Milestone

Comments

@danwatford
Copy link
Contributor

Category

[X] Enhancement

[ ] Bug

[ ] Question

Version

Please specify what version of the library you are using: [ 3.7.2 ]

Desired Behavior

The Toolbar control currently maintains the set of selected filters as internal state, and communicates changes in this internal state to its parent component via calls to a function passed as the onSelectedFiltersChange property.

I would like to use Toolbar as a controlled component where the set of selected filters is maintained externally.

The usecase for this is to set the initial set of filters that correspond to the default view of a list, and to allow changes to the current set of selected filters based on interaction with other UI components, such as dashboard indicators.

Observed Behavior

The onSelectedFiltersChange function does provide a way for the set of selected filters to be influenced externally. This function is called whenever the user interacts with the rendered filter tree or the Clear filters button with the new set of selected filters. The returned array from this function is used by the Toolbar to set the set of selected filters.

We can use our implementation of the onSelectedFiltersChange function to enable filters that the user has not manually activated. However this function is only called in response to a user interaction with the filter tree.

Initial implementation available

I have tested a basic implementation and can create a PR. It boils down to a change in ToolbarFilter.tsx similar involving a new optional prop, selectedFilterIds and a useEffect to apply changes in selectFilterIds to the internal filter state:

export const ToolbarFilter = (props: IExtendedToolbarFilterProps) => {
  const {
    layout,
    filters,
    selectedFilterIds: externalSelectedFilters,
    singleSelect,
    open,
    onOpenChange,
    toolbarMenuProps,
  } = props;
  const [selectedFilters, setSelectedFilters] = React.useState<string[]>([]);
  const propagateSetSelectedFilters = (eventSelectedFilters: string[]) =>
    setSelectedFilters(
      (props.onSelectedFiltersChange || passThrough)(eventSelectedFilters)
    );

  React.useEffect(() => {
    if (externalSelectedFilters) {
      setSelectedFilters(externalSelectedFilters);
    }
  }, [externalSelectedFilters]);

The above allows the control to be used in both controlled and uncontrolled modes.

@ghost
Copy link

ghost commented May 25, 2022

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

@ghost ghost added the Needs: Triage 🔍 label May 25, 2022
@github-actions
Copy link

Thank you for submitting your first issue to this project.

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jun 7, 2022

@danwatford - thanks for suggestion!
Please, feel free to submit the PR!

@AJIXuMuK AJIXuMuK added type:enhancement New feature or enhancement of existing capability and removed Needs: Triage 🔍 labels Jun 7, 2022
danwatford added a commit to danwatford/sp-dev-fx-controls-react that referenced this issue Jun 13, 2022
@joelfmrodrigues joelfmrodrigues added this to the 3.9.0 milestone Jun 21, 2022
@joelfmrodrigues joelfmrodrigues added the status:fixed-next-drop Issue will be fixed in upcoming release. label Jun 21, 2022
@AJIXuMuK AJIXuMuK mentioned this issue Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:fixed-next-drop Issue will be fixed in upcoming release. type:enhancement New feature or enhancement of existing capability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants