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

Sample gallery with extension type, Closes #219 #260

Merged
merged 13 commits into from
Aug 25, 2024

Conversation

nicodecleyre
Copy link
Contributor

@nicodecleyre nicodecleyre commented Jun 18, 2024

🎯 Aim

Adding an extra filter to Sample gallery so that we can filter on the extension type. Filter can only be visible when extension type is selected 'Component Type'

πŸ“· Result

image

βœ… What was done

  • Added Extension filter dropdown
  • Added functionality so that extension filter is only visible when extension is selected in the Component Type dropdown
  • Added filter functionality so that samples get filtered based on selected value in extension dropdown
  • Added filter functionality so that samples don't get filtered when extensions dropdown has selected values but extension is not selected in component type dropdown

πŸ”— Related issue

Closes: #219

@nicodecleyre nicodecleyre force-pushed the Sample-gallery-with-extension-type-#219 branch from e229636 to ee0f3fe Compare June 18, 2024 19:11
@Adam-it Adam-it self-assigned this Jun 18, 2024
@Adam-it
Copy link
Contributor

Adam-it commented Jun 18, 2024

Awesome πŸ‘πŸ‘πŸ‘πŸ‘ I will review it ASAP 🀩
You Rock 😍

Copy link
Contributor

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@nicodecleyre awesome work so far πŸ‘
The implementation is really good and clean and I have only some small details I commented.
The biggest problem I noticed is a small bug that:
I managed to have the extension type filter present not only with extension component type but with webpart as well. Then I selected form customizer type and when I unselected and selected extension component type again it seemed that the form customizer extension type was already selected (the samples were filtered) but it wasn't selected in the picker. Please check the below recording for more details.
https://github.com/pnp/vscode-viva/assets/58668583/0866d980-6d59-4653-8183-745c901cb0eb

src/vscode.d.ts Outdated Show resolved Hide resolved
src/webview/view/components/gallery/SearchBar.tsx Outdated Show resolved Hide resolved
src/webview/view/hooks/useSamples.tsx Outdated Show resolved Hide resolved
src/constants/Prompts.ts Outdated Show resolved Hide resolved
src/webview/view/components/gallery/SearchBar.tsx Outdated Show resolved Hide resolved
@Adam-it Adam-it marked this pull request as draft June 29, 2024 22:50
@nicodecleyre nicodecleyre force-pushed the Sample-gallery-with-extension-type-#219 branch from 43e46e8 to 6af7cf1 Compare August 2, 2024 08:25
@nicodecleyre nicodecleyre marked this pull request as ready for review August 2, 2024 09:35
Copy link
Contributor

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@nicodecleyre the implementation looks ok but when testing it locally it seems like it is a bit inconsistent with the rest of filters and some behavior is still buggy

Please check below recording for more context.

PR.test.mp4
  • it seems the selected extension type filter are not pointed out in the text box and the checkbox are not marked
  • the picked option are not shown in the tag below as other filters
  • when we 'clear all' the extension gets unselected but the extension type select gets not hidden

@Adam-it Adam-it marked this pull request as draft August 5, 2024 21:33
@nicodecleyre nicodecleyre force-pushed the Sample-gallery-with-extension-type-#219 branch from 69f8ce4 to 412d3b5 Compare August 8, 2024 09:14
@nicodecleyre
Copy link
Contributor Author

@nicodecleyre the implementation looks ok but when testing it locally it seems like it is a bit inconsistent with the rest of filters and some behavior is still buggy

Please check below recording for more context.

PR.test.mp4

  • it seems the selected extension type filter are not pointed out in the text box and the checkbox are not marked
  • the picked option are not shown in the tag below as other filters
  • when we 'clear all' the extension gets unselected but the extension type select gets not hidden

Thx @Adam-it, indeed a bit inconsistent like you said. Did a couple of changes and tested. Seems to work fine on my end.

Let me know if you find other bugs πŸ™

@nicodecleyre nicodecleyre marked this pull request as ready for review August 10, 2024 16:55
Copy link
Contributor

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

Looks and works really good πŸ‘
Added some minor styling tweaks.
Awesome work πŸ‘ You Rock 🀩

@Adam-it Adam-it merged commit 11100a4 into pnp:dev Aug 25, 2024
Adam-it pushed a commit that referenced this pull request Sep 8, 2024
## 🎯 Aim

Adding an extra filter to Sample gallery so that we can filter on the
extension type. Filter can only be visible when extension type is
selected 'Component Type'

## πŸ“· Result

![image](https://github.com/pnp/vscode-viva/assets/35696168/c8c2e084-5831-4fc4-861e-ad4c4c0845d7)

## βœ… What was done

- [X] Added Extension filter dropdown
- [X] Added functionality so that extension filter is only visible when
extension is selected in the Component Type dropdown
- [X] Added filter functionality so that samples get filtered based on
selected value in extension dropdown
- [x] Added filter functionality so that samples don't get filtered when
extensions dropdown has selected values but extension is not selected in
component type dropdown

## πŸ”— Related issue

Closes: #219
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.

2 participants