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

Recipe filters #1732

Merged
merged 7 commits into from
Dec 12, 2024
Merged

Recipe filters #1732

merged 7 commits into from
Dec 12, 2024

Conversation

fredex42
Copy link
Contributor

@fredex42 fredex42 commented Nov 19, 2024

What's changed?

Adds options to specify "celebration" and "suitable for diet" filters.

Implementation notes

These are currently simple dropdown options, we should probably look at something more generic and sophisticated but this works at the moment

Checklist

General

  • 🤖 Relevant tests added
  • ✅ CI checks / tests run locally
  • 🔍 Checked on CODE

Client

  • 🚫 No obvious console errors on the client (i.e. React dev mode errors)
  • 🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
  • 📷 Screenshots / GIFs of relevant UI changes included

@fredex42 fredex42 requested a review from a team as a code owner November 19, 2024 16:46
@fredex42 fredex42 marked this pull request as draft November 19, 2024 16:47
@fredex42 fredex42 marked this pull request as ready for review December 11, 2024 12:26
Copy link
Contributor

@jonathonherbert jonathonherbert left a comment

Choose a reason for hiding this comment

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

LGTM! An endpoint per keyword seems a lot, but perhaps we're worried about how this data grows, and it eliminates a class of problem.

This is going to be really useful.

It'd be lovely to display the doc counts alongside the option values, to give users an idea of how many recipes they're likely to see for a given option — guiding filtering behaviour and potentially saving some clicks. (Ideally, they'd be running totals that would update for combinations of options, but that's probably a larger piece of work.)

@fredex42 fredex42 enabled auto-merge December 12, 2024 10:25
@fredex42 fredex42 merged commit a1b08de into main Dec 12, 2024
13 checks passed
@fredex42 fredex42 deleted the ag/recipe-filters branch December 12, 2024 10:29
@prout-bot
Copy link

Seen on PROD (merged by @fredex42 6 minutes and 25 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants