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

Plugins: Fix plugin catalog filtering #66663

Merged
merged 2 commits into from
Apr 18, 2023

Conversation

leventebalogh
Copy link
Contributor

@leventebalogh leventebalogh commented Apr 17, 2023

Fixes #66478

What changed?

Updated the filtering in the plugins catalog to be able to use the different filters together. Also added some tests for the find() selector.

Screen.Recording.2023-04-17.at.13.34.28.mov

@leventebalogh leventebalogh added this to the 10.0.0 milestone Apr 17, 2023
@leventebalogh leventebalogh self-assigned this Apr 17, 2023
@leventebalogh leventebalogh marked this pull request as ready for review April 17, 2023 11:35
@leventebalogh leventebalogh requested a review from a team as a code owner April 17, 2023 11:35
@leventebalogh leventebalogh requested review from andresmgot, jackw and sympatheticmoose and removed request for a team April 17, 2023 11:35
Copy link
Contributor

@andresmgot andresmgot left a comment

Choose a reason for hiding this comment

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

LGTM! I suggest adding another test but don't need to block this for that

@@ -55,7 +55,7 @@ export default function Browse({ route }: GrafanaRouteComponentProps): ReactElem
};

const onSearch = (q: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if it would be too complicated but it would be nice to have a test that verifies that the bug is gone with the react testing library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yepp, I think that's a good point, will add another test case in the afternoon 👍

Copy link
Contributor

@sympatheticmoose sympatheticmoose left a comment

Choose a reason for hiding this comment

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

LGTM! Tested locally and behaves as expected now 🥇

@leventebalogh leventebalogh enabled auto-merge (squash) April 18, 2023 06:07
@leventebalogh leventebalogh merged commit c22c31e into main Apr 18, 2023
11 checks passed
@leventebalogh leventebalogh deleted the leventebalogh/plugin-catalog-fix branch April 18, 2023 06:15
@@ -20,6 +20,7 @@ const useDebounceWithoutFirstRender = (callBack: () => any, delay = 0, deps: Rea
isFirstRender.current = false;
return;
}
console.log('--------- DEBUOUNCE');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you forgot to remove these @leventebalogh 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quickfix for removing them 😅
#66725

@zerok zerok modified the milestones: 10.0.0, 10.0.0-preview May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Type filter doesn't work when combined with string search
5 participants