Feature/client/recipe overlay

[hagaishapira]

Yeah that's fine, just want to keep track of this with an issue.

On Tue, Jun 2, 2020, 12:50 guyschwartzberg-student notifications@github.com wrote:

@guyschwartzberg-student commented on this pull request.

In api/search.js https://github.com/recipeify/Recipes/pull/43#discussion_r433756232:

• gte: fromCookTime,
• lte: toCookTime,
• body.query.bool.filter = [
• {
• bool: {
• should: [
• {
• range: {
• total_time: {
• gte: fromCookTime,
• lte: toCookTime,
• },
• },
• },
• {
• bool: {

I think Noa can overwrite this when she commits her change, for now I think it is wise to show as many recipes as possible as long as we don't have the sliders yet

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/recipeify/Recipes/pull/43#discussion_r433756232, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG6SIFZZUK4VO3XF5GKNRJTRUTDO5ANCNFSM4NQSACDQ .

[hagaishapira]

@guyschwartzberg-student add at client/.eslintrc.json at the end in "rules"

"rules": {   
          "no-console": "off",
    }

This should at least cleanup all console use warnings

[hagaishapira]

@guyschwartzberg-student Couple of issues I see when playing with this

• clicking on image doesn't send me to the external site
• clicking on external site button doesn't send me to external site
• image in expanded view is not scaled properly

[guyschwartzberg-student]

@hagaishapira your CSS is probably not updated, dunno why it happened but the image shouldn't do that (It's not on my screen), likewise the external button sends me to the site. Also I am not sure about if the image should send us to the site.

[hagaishapira]

@guyschwartzberg-student additionally, why not implement the save recipes call already for the star buttons?

[hagaishapira]

I'll check my CSS, don't know how it could have happened. Clicking on the image should definitely do something. The options are expanding or sending to external site. I think the case for external site is stronger since I think our value is in connecting the users to recipes as quickly as possible.

On Tue, Jun 2, 2020, 13:24 guyschwartzberg-student notifications@github.com wrote:

@hagaishapira https://github.com/hagaishapira your CSS is probably not updated, dunno why it happened but the image shouldn't do that (It's not on my screen), likewise the external button sends me to the site. Also I am not sure about if the image should send us to the site.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/recipeify/Recipes/pull/43#issuecomment-637444615, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG6SIF2X3ZE3Y55PZRUDE23RUTHNJANCNFSM4NQSACDQ .

[guyschwartzberg-student]

I wanted to implement the save recipe to cook book but it requires a bit of redux (Because you want recipes that have already been saved to show that to the user while searching) and I am still unsure about it.

[hagaishapira]

@guyschwartzberg-student

• Okay so now the image is clickable but not the card body itself, I think they should both have the same action.
• I think we should display the site name and just the rating (without the number of raters) under the title in the card.
• I think there is a bug of not showing the number of raters

[hagaishapira]

@guyschwartzberg-student plus, can you go over and fix the linting issues?

[guyschwartzberg-student]

I am against making the entire card clickable, I don't think that'd be a good idea. That's what the buttons are for, after all.

By under the card, do you mean in the gallery or the modal?

And I fixed the eslint issues and the bug you mentioned,, will push after your response :)

[hagaishapira]

Oops, I meant in the card meta description.

I disagree about the clickability. If the image is clickable then the body should as well. It makes it weird otherwise. @LeshemNoa what do you think?

On Tue, Jun 2, 2020, 15:45 guyschwartzberg-student notifications@github.com wrote:

I am against making the entire card clickable, I don't think that'd be a good idea. That's what the buttons are for, after all.

By under the card, do you mean in the gallery or the modal?

And I fixed the eslint issues and the bug you mentioned,, will push after your response :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/recipeify/Recipes/pull/43#issuecomment-637517855, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG6SIFYYCKNFE6FEURCFCZ3RUTX5JANCNFSM4NQSACDQ .

[guyschwartzberg-student]

Personally I think neither should be clickable. There are two seperate buttons (one on the card, one in the modal) to navigate to the external website. @LeshemNoa what do you think?

[hagaishapira]

@guyschwartzberg-student for the final eslint errors update client/.eslintrc.json

    "env": {
        "browser": true,
        "es6": true,
        "jest": true
    },

[guyschwartzberg-student]

Still seems to fail, I am not sure what the problem is

[hagaishapira]

Still seems to fail, I am not sure what the problem is

Well it says what it is :) Change App.test