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

cancel search link, fix p.2 from #1850 #1922

Closed
wants to merge 5 commits into from
Closed

cancel search link, fix p.2 from #1850 #1922

wants to merge 5 commits into from

Conversation

keram
Copy link
Contributor

@keram keram commented Sep 3, 2012

@parndt ? :)

@@ -50,8 +50,9 @@ en:
current: Current File
search:
button_text: Search
results_for: "Search results for '%{query}'"
results_for: "Search results for &#8216;<em>%{query}</em>&#8217;"
Copy link
Member

Choose a reason for hiding this comment

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

This key should be results_for_html now given it contains HTML.

@parndt
Copy link
Member

parndt commented Sep 3, 2012

Can you please put a before/after screenshot here so that we have the history of what this did?

Thanks :)

@travisbot
Copy link

This pull request passes (merged ea2d36e into 0b1de8b).

@keram
Copy link
Contributor Author

keram commented Sep 3, 2012

captain here are screenshots
before
after

@travisbot
Copy link

This pull request passes (merged 1edcdc4 into 0b1de8b).

@travisbot
Copy link

This pull request passes (merged fcbc5fa into 0b1de8b).

@ugisozols
Copy link
Member

Looks good.

@ugisozols
Copy link
Member

Unfortunately I can't comment inline so I'm putting some code here.

In the engine generator template (2nd line) there's code:

refinery.admin_<%= plural_name %>_path

The link won't work because engine routes are namespaced. You should use:

refinery.<%= namespacing.underscore %>_admin_<%= plural_name %>_path

@ugisozols
Copy link
Member

One more thing - changes to css makes the backed look a bit different than it is now:

original:
before

after changes:
after

Please fix that and also do a squash. Thanks :)

@keram
Copy link
Contributor Author

keram commented Sep 7, 2012

@ugisozols please check if is look good for you now.
I tested this only on ubuntu and on other platforms may been still some small issues

@keram keram mentioned this pull request Sep 7, 2012
@ugisozols
Copy link
Member

@keram I squashed your commits and merged this in 1a35085 + updated other locales here - a75309d

@ugisozols ugisozols closed this Sep 10, 2012
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.

4 participants