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 pluralization to ingredient displays #595

Merged
merged 2 commits into from Nov 3, 2022

Conversation

lortza
Copy link
Owner

@lortza lortza commented Nov 2, 2022

I thought it was time to make the ingredient display look smarter with pluralization. The tricky part is that sometimes we pluralize the measurement unit, like "3 cups water" but other times we pluralize the food name, like "3 medium carrots". This PR handles that logic for the recipe show page and the meal plan show page.

Screenshots

Before

The recipe show page does not pluralize any part of the ingredient listing.
Screen Shot 2022-11-02 at 6 55 19 PM

The meal plan show page does not pluralize any part of the ingredient listing.
Screen Shot 2022-11-02 at 6 57 35 PM

After

The recipe show page pluralizes ingredients when there is a descriptive measurement unit, like "large" or "whole" and pluralizes the measurement unit for normal units like "cup" or "tablespoon".
Screen Shot 2022-11-02 at 6 55 28 PM

The meal plan show page pluralizes in the same pattern as the recipe show page. Also, the link to the recipe is now from only the recipe title text.
Screen Shot 2022-11-02 at 6 57 05 PM

Things Learned

  • View helpers are ugly. This PR has some repeated logic and it looks clunky, but I think it is okay for now.
  • Making small pretty changes is very satisfying.

Due Diligence Checks

  • I have written new specs for this work
  • I have browser tested this work
  • I have deployed the feature branch to production and browser tested it successfully

@@ -6,6 +6,5 @@
sequence(:name) { |n| "ingredient name #{n}" }
quantity { 1.5 }
measurement_unit { Ingredient::UNITS[1..-1].sample }
preparation_style { Ingredient::STYLES[1..-1].sample }
Copy link
Owner Author

Choose a reason for hiding this comment

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

preparation_style is not a required field, so it should not be in the factory default.

@@ -43,6 +43,7 @@
<% details.each do |detail| %>
<li>
<%= detail_display(detail) %>
(<%= link_to detail.recipe.title, detail.recipe %>)
Copy link
Owner Author

@lortza lortza Nov 3, 2022

Choose a reason for hiding this comment

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

isolates the link to the recipe page to only the recipe title text and effectively moves the html out of the helper method.

].join(' ')

link_to ingredient, recipe_path(detail.recipe)
pluralized = if Ingredient::DESCRIPTIVE_UNITS.include?(detail.measurement_unit)
Copy link
Owner Author

@lortza lortza Nov 3, 2022

Choose a reason for hiding this comment

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

Used in the Meal Plan show page to pluralize like:

  • 6 whole, chopped
  • 3 cloves, minced


if ingredient.preparation_style.present?
"#{display_name}: #{ingredient.preparation_style}"
display_name = if Ingredient::DESCRIPTIVE_UNITS.include?(ingredient.measurement_unit)
Copy link
Owner Author

Choose a reason for hiding this comment

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

Used in the Recipe show page to pluralize like:

  • 6 whole carrots, chopped
  • 3 cloves garlic, minced

Comment on lines +44 to +47
UNITS = [
STANDARD_UNITS,
DESCRIPTIVE_UNITS,
].flatten.sort
Copy link
Owner Author

Choose a reason for hiding this comment

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

Breaks measurement units into groups based on pluralization needs, then ensures wherever UNITS is used that all items appear in alpha order.

@lortza lortza temporarily deployed to myfoodplanner November 3, 2022 14:26 Inactive
@lortza lortza merged commit 3450097 into main Nov 3, 2022
@lortza lortza deleted the add_pluralization_to_ingredient_displays branch November 3, 2022 14:31
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.

None yet

1 participant