Needs Feedback: Homepage redesign redux #697
5 participants
tygriffin
commented on an outdated diff
| @@ -61,14 +61,20 @@ def hierarchy | ||
| # GET /crops/search | ||
| def search | ||
| @all_matches = Crop.search(params[:search]) | ||
| - exact_match = Crop.find_by_name(params[:search]) | ||
| + exact_match = Crop.find_by(name: params[:search]) | ||
| + exact_search = params[:exact_search] || false | ||
| + | ||
| if exact_match | ||
| @all_matches.delete(exact_match) |
tygriffin
This seems to delete from the actual database in development. I think this is because we're not using Elasticsearch in the dev env and so
Not sure how to debug this. |
|
Does this still need a review? |
|
@cesy : I believe it does. |
| + | ||
| +.title-banner { | ||
| + position: relative; | ||
| + background-image: image-url('veggies.jpg'); | ||
| + background-repeat: no-repeat; | ||
| + background-size: 100%; | ||
| + min-height: 280px; | ||
| + padding: 30px; | ||
| + margin: 0 -15px 0 -15px; | ||
| + | ||
| + @media (max-width: @screen-md-min) { | ||
| + background-size: @screen-md-min; | ||
| + } | ||
| + | ||
| + h1 { | ||
| + font-size: 60px; |
Skud
Actually, can we use Bootstrap less variables please! Bootstrap itself is based on a base pixel size, and then has things like On 27/03/2015 12:09 am, Taylor Griffin wrote:
Alex "Skud" Bayley
tygriffin
So glad you made this suggestion. I've been using variables for all kinds of padding and margins at work, and it's been a real game changer. I'll get caught up on this PR this weekend, btw. |
|
Is it easy to split out the search changes into a separate PR? |
|
Can we also add a feature test for the new homepage, to check some of the values and features are coming out correctly? I've had a first review of this and most of it looks good. It will need someone with more Javascript expertise to review those parts. |
|
@cesy : Check and check on the search changes and feature tests. And speaking of separate PRs, I think we should break out the redesign of the login page into a separate story as well, and just make sure it's included in the same PR as this one. I can probably whip this PR into shape, but I'm too swamped with work / life stuff at the moment to take on too much more. |
|
Hi @tygriffin, we've now got Release 8 out the way and are keen to get this moving again if you're around. Sorry for the delay! |
|
Hi @cesy! Great to hear about Release 8. Unfortunately, I don't think I'll have time to do much development on Growstuff in the foreseeable future. Do you think someone might be able to take over this story? |
|
I'd be happy to take this one over and redo against current dev |
|
Thank you, that would be great! |
|
Shifted to #1113 |
Fixes #628
Replaces #674
@Skud :> I think after you select something from that search UI it should take you direct to the crop page in question (eg. /crops/eggplant) not to search results (unless there's not an exact match).This should work in production where we're using Elasticsearch but it doesn't seem to work in development. See inline note.@Skud :
Good call. I need to refactor autosuggest a bit. Should I do that now or break into a new story?
Question: Should the signed in user homepage redesign be part of this PR?
Screenshot (updated March 1, 2015)
