Added fix for Issue 59: Instructions to search for items #68
Conversation
|
@mehulsr here is a pointer for linking to the README: https://www.playframework.com/documentation/2.5.x/JavaTemplates#Escaping |
| @@ -12,6 +12,8 @@ | |||
|
|
|||
| <h2>@message("searchItems")</h2> | |||
|
|
|||
| <p>@message("searchInstructions")</p> | |||
There was a problem hiding this comment.
Please be sure to use spaces rather than tabs for indentation.
If you're using Eclipse, here is a guide for configuring it to use spaces: http://stackoverflow.com/a/408449/29470
There was a problem hiding this comment.
Thanks, will fix this in the next commit.
I'm not using any IDE at the moment, but the Atom Editor. Is it advisable to use an IDE?
There was a problem hiding this comment.
Fixed this in new commit.
web-gateway/conf/messages
Outdated
| @@ -85,3 +85,6 @@ invalid.duration=Please enter a valid duration | |||
| invalid.units=Please enter a valid unit | |||
|
|
|||
| createUserInstructions=Here you can create a new user who can participate in and create auctions. | |||
| searchInstructions=Here you can search for auctioned items. Search functionality can be enabled by downloading and running a local instance of elasticsearch. \ | |||
| Steps for doing this can be found in README.md.\ | |||
| You can filter your search by specifying the maximum price of items to search for and the currency to search in. | |||
There was a problem hiding this comment.
Hi @mehulsr, thanks for this contribution.
I have a question: are the newlines preserved when rendering this message in HTML? Or put another way, will this message be rendered as 3 <p> blocks? If not, is there
I think we could rewrite them so that the generic instruction on how to use the feature appear first (since they always apply), and the details on how to get the feature up and running (install ES and/or check README) appear last. Also, I think it's worth mentioning that once the system is started, there's no need to restart it when installing and running ES since the search service will be able to recover.
There was a problem hiding this comment.
Hi @ignasi35
The newlines are not preserved at the time of rendering in HTML.
I shall reorder the instructions in the next commit to separate general search instructions and installation instructions.
There was a problem hiding this comment.
Fixed this in new commit.
|
Do you still intend to add a link to |
|
@TimMoore Yes I think it would be appropriate. I'll try to do it one more time, else we can close the PR? |
|
OK no worries, just let us know when you're ready for us to merge it. |
|
Yes, sure 😃 |
mehulsr
force-pushed
the
mehulsr-fix-Issue59
branch
from
21c3bab
to
50550c0
Compare
|
@TimMoore @ignasi35 |
web-gateway/conf/routes
Outdated
| @@ -22,3 +22,5 @@ GET /search controllers.SearchController.searchForm( | |||
| POST /search controllers.SearchController.search() | |||
|
|
|||
| GET /assets/*file controllers.Assets.at(path = "/public", file) | |||
|
|
|||
| GET /*file controllers.ExternalAssets.at(path="../", file) | |||
There was a problem hiding this comment.
hi @mehulsr we can't do this. With this route I can download any file in the folder including the source code (test http://localhost:9000/LICENSE or http://localhost:9000/build.sbt for example).
I think we can safely link to https://github.com/lagom/online-auction-java/blob/master/README.md on the instructions (in which case this route won't be necessary).
There was a problem hiding this comment.
Ah, yes that's true. Okay, pushing with this change now
|
Looks great! Thanks @mehulsr |
Fixes #59
Trying to figure out how to add link to README.md