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

Add tag filtering #3165

Merged
merged 32 commits into from
Feb 26, 2024

Conversation

G-Ambatte
Copy link
Collaborator

This PR resolves #3164.

It adds the updateListFilter function to the BrewItem component, allowing it to update the filter in the upstream ListPage.

This function currently takes two arguments, type and term. In the current implementation, type is unused, and term is applied to the existing text filter.

@G-Ambatte G-Ambatte marked this pull request as ready for review December 3, 2023 00:37
@G-Ambatte G-Ambatte self-assigned this Dec 3, 2023
@G-Ambatte G-Ambatte added the P2 - minor feature or not urgent Minor bugs or less-popular features label Dec 3, 2023
@G-Ambatte G-Ambatte changed the title Experimental tag filtering #3164 Add tag filtering Dec 3, 2023
@ericscheid
Copy link
Collaborator

Looking great, all we need is UI feedback/status to show that the userpage is currently filtered by tags.

I'm imagining a 2nd userpage utility bar that appears when tag filtering is in effect.

image
  • Clicking any of those tags should unselect that tag from the filter.
  • They should be sorted and styled similar to how tags are in .brewItem

@calculuschild calculuschild temporarily deployed to homebrewery-pr-3165 January 17, 2024 17:22 Inactive
@calculuschild calculuschild temporarily deployed to homebrewery-pr-3165 January 18, 2024 20:04 Inactive
@G-Ambatte G-Ambatte added 🔍 R4 - Reviewed - Fixed! ⭐ PR review comments addressed and removed 🔍 R3 - Reviewed - Awaiting Fixes 🔧 PR is okayed but needs fixes before merging labels Feb 22, 2024
@5e-Cleric 5e-Cleric self-requested a review February 22, 2024 07:40
@5e-Cleric
Copy link
Member

5e-Cleric commented Feb 24, 2024

Remove that last console log and i think this is done, merging in 24 hours if nobody says otherwise.

@5e-Cleric 5e-Cleric added this to the V3.12 milestone Feb 24, 2024
Copy link
Member

@calculuschild calculuschild left a comment

Choose a reason for hiding this comment

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

Trusting 5e-Cleric has done his due dilligence on this. Just noticed one thing I'm not sure about.

client/homebrew/pages/basePages/listPage/listPage.jsx Outdated Show resolved Hide resolved
@5e-Cleric
Copy link
Member

Trusting 5e-Cleric has done his due dilligence on this. Just noticed one thing I'm not sure about.

Gotta be honest, not sure how this one can go sideways, or how to test it other than using it, i already reported one issue, but it is outside of the scope of this PR, and taking care of it in #3284, albeit could use some help with that.

@calculuschild calculuschild temporarily deployed to homebrewery-pr-3165 February 26, 2024 08:17 Inactive
@G-Ambatte G-Ambatte merged commit bc29cdd into naturalcrit:master Feb 26, 2024
2 checks passed
@G-Ambatte G-Ambatte deleted the experimentalTagFiltering-#3164 branch February 26, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 - minor feature or not urgent Minor bugs or less-popular features 🔍 R4 - Reviewed - Fixed! ⭐ PR review comments addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filter brews by clicking tags
4 participants