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

Add Indication To Search #10956

Merged
merged 2 commits into from
Apr 3, 2021
Merged

Add Indication To Search #10956

merged 2 commits into from
Apr 3, 2021

Conversation

MatthewKennedy
Copy link
Contributor

@MatthewKennedy MatthewKennedy commented Apr 2, 2021

Add a little clue to what each search filed is intended to be used for, example:
Search by: Order Number
Search by: Product Name

Issus Fixed in this PR:

  • Duplicate key in en.yml
  • Missing js-filterable js-quick-search-target class causing the ransack search and filters panel to be unusable on stock transfers index.
naming.mov

@MatthewKennedy
Copy link
Contributor Author

I should imagine this will fail as tests will be looking for less descriptive label name in order and products fields.

@@ -1281,6 +1281,7 @@ en:
price_sack: Price Sack
process: Process
product: Product
product_name: Product Name
Copy link
Member

Choose a reason for hiding this comment

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

@MatthewKennedy I think we can avoid adding a new translation key by combining product + name translation here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah sure, I'll sort that.

Copy link
Contributor Author

@MatthewKennedy MatthewKennedy Apr 3, 2021

Choose a reason for hiding this comment

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

@damianlegawiec I'll drop that back to just :name if you want?

I was just thinking then as I was concatenating keys, if we build a string from separate translation keys, it might not translate well out of its context of use, if you were to use an auto translation service like translation.io on your app.

Product number => Numéro de produit
product + number => Produit Numéro

What would you prefer? I can concat the keys, drop it back to just :name or leave it with the new translation key?

Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it as product_name then as it's best for UX 👍

@@ -1623,7 +1625,6 @@ en:
customer_support_email_help: "This email is visible to your Store visitors in the Footer section"
new_order_notifications_email_help: "If you want to receive an email notification every time someone places an Order please provide an email address for that notification to be sent to"
seo_robots: "Please check <a href='https://developers.google.com/search/reference/robots_meta_tag' target='_blank'>this page for more help</a>"
store_set_default_button: Set as default
Copy link
Member

Choose a reason for hiding this comment

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

@MatthewKennedy shouldn't this be kept?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats a duplicate key found:

Screenshot 2021-04-03 at 14 06 04

Copy link
Contributor Author

Choose a reason for hiding this comment

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

YML linter was telling me duplicate key.

@damianlegawiec damianlegawiec merged commit 3cbd10d into spree:master Apr 3, 2021
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.

None yet

2 participants