Shows default image in the frontend #91
Conversation
| showRecipeImageThumb: function() { | ||
| if (this.props.data.photo_thumbnail) { | ||
| return ( | ||
| <img className="img-responsive" src={ this.props.data.photo_thumbnail } /> |
There was a problem hiding this comment.
I think we should also show the default image here as well. Just doesn't look as good without it, IMO.
| id: 'recipe.photo_placeholder', | ||
| description: 'Photo placeholder', | ||
| defaultMessage: 'Generic placeholder thumbnail' | ||
| } | ||
| }); | ||
|
|
||
| return ( | ||
| <div className="recipe-details"> | ||
| <h1 className="title hidden-xs">{this.props.data.title}</h1> |
There was a problem hiding this comment.
Should we only show this header if there is an Image? Looks weird to have it twice so quickly. Thoughts?
There was a problem hiding this comment.
Good point, but now that I've had a good look, I'm not convinced the current layout makes a lot of sense. If the image exists, we show the image twice, which looks kinda weird. And if there is no image, then we show the title of the recipe in a different place and a useless placeholder. I feel like we should either just show the large image first and no first header or thumbnail. Or just the thumbnail (with a placeholder if no image) and have the green title always be the header. Or maybe the large image appears under the green title?
I'm going to remove the first title if the image doesn't exist for the time being, and we can open a separate issue to see if we can improve this page.
There was a problem hiding this comment.
I'm by no means a UX/UI expert. If you have some ideas, I'd love to see them
hermzz
force-pushed
the
default-recipe-images
branch
2 times, most recently
from
c55a317
to
78be059
Compare
hermzz
force-pushed
the
default-recipe-images
branch
from
78be059
to
0d55c5f
Compare
|
Good point, deleted! |
|
Maybe I'm crazy but I'm not seeing the image as deleted. Did you forget to push a commit? |
hermzz
force-pushed
the
default-recipe-images
branch
from
adc6c16
to
8a47dc9
Compare
|
Not crazy, I'm a moron. Commit pushed for reals this time. |
Because finding a new logo image is outside of the scope of this PR I've just created a placeholder for the time being.
Fixes #82.