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
Goals plugin language reworked #19307
base: 4.x-dev
Are you sure you want to change the base?
Conversation
Thanks @comradekingu. These changes including your self-review look good. If you commit those and the fixes I've added we can merge this PR. |
Co-authored-by: Justin Velluppillai <justinvelluppillai@gmail.com>
@justinvelluppillai |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a closer look through the changes and left a couple of comments for improvements.
plugins/Goals/lang/en.json
Outdated
"CaseSensitive": "Case sensitive match", | ||
"BestCountries": "The countries with the most conversions are:", | ||
"BestKeywords": "The keywords with the most conversions are:", | ||
"BestReferrers": "The websites sending the most referrers are:", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one actually now has another meaning. It should be more something like The referring websites with the most conversions are:
plugins/Goals/lang/en.json
Outdated
"ColumnConversionRatePageViewedBeforeDocumentation": "The percentage of all conversions for goal %s where this page was viewed before conversion.", | ||
"ColumnAverageQuantityDocumentation": "The average quantity of this %s sold in e-commerce orders.", | ||
"ColumnConversionRateDocumentation": "The percentage of visits that triggered the %s goal.", | ||
"ColumnConversionRatePageViewedBeforeDocumentation": "The percentage of all conversions for the %s goal where this page was viewed before conversion.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder will be filled with the name of the goal. Not sure if the change makes sense in that case.
"ColumnConversionRatePageViewedBeforeDocumentation": "The percentage of all conversions for the %s goal where this page was viewed before conversion.", | |
"ColumnConversionRatePageViewedBeforeDocumentation": "The percentage of all conversions for goal %s where this page was viewed before conversion.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only way to do that in English is to make it
for the goal "%s"
, which is a bit unnatural wrt. verb, subject, object.
https://ejournals.lib.auth.gr/thal/article/view/5443 https://www.ilovelanguages.com/do-other-languages-have-inverted-verb-order-like-german/
Or "the goal of getting %s done", or something along those lines.
"ColumnOrdersDocumentation": "The total number of Ecommerce orders which contained this %s at least once.", | ||
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the sum of Product quantities sold in all Ecommerce orders.", | ||
"ColumnOrdersDocumentation": "The total number of e-commerce orders which contained this %s at least once.", | ||
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the sum product quantity of all e-commerce orders sold.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
English is not my main language, but for me it sounded better like it was before. sum product quantity
somehow sounds incorrect. Shouldn't that be either sum of product quantities
or summed up product quantity
🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgiehl The second "Product quantities" is redundant, and "Product" should have a small "p". "Ecommerce" is also wrong.
The transitive verb "sum", like "sum total" is better than "quantities sold in all orders", as that is very dubious English.
plugins/Goals/lang/en.json
Outdated
"DaysToConv": "Days to Conversion", | ||
"DaysToConvReportDocumentation": "This report shows how many days pass before your visitors convert a goal.", | ||
"DaysToConvReportDocumentation": "This report shows how many days pass on average before your visitors convert a goal.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plugins/Goals/lang/en.json
Outdated
"HelpOneConversionPerVisit": "If a Page matching this Goal is refreshed or viewed more than once in a Visit, the Goal will only be tracked the first time the page was loaded during this visit.", | ||
"GoalsOverview": "Overview of Goals", | ||
"GoalsOverviewDocumentation": "An overview of your goal conversions. Initially, the graph shows the sum of all conversions. %s Below the graph, you can see conversion reports for each of your goals. The sparklines can be enlarged by clicking on them.", | ||
"GoalX": "%s Goal", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder will also be replaced with the name of the goal
"GoalX": "%s Goal", | |
"GoalX": "Goal %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use quotes here then as well.
plugins/Goals/lang/en.json
Outdated
"NewVisitorsConversionRateIs": "New visitors conversion rate is %s", | ||
"NoGoalsNeedAccess2": "Only a Write user, an Administrator or a user with Super User access can manage Goals for a given website. Please ask your Matomo administrator to set up a Goal for your website. <br>Tracking Goals is a great way to help understand and maximize your website performance!", | ||
"ManuallyTriggeredUsingJavascriptFunction": "A goal is manually triggered using the JavaScript API trackGoal()", | ||
"MatchesExpression": "matches the %s expression", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder contains a expression that was defined by the user like /test/
"MatchesExpression": "matches the %s expression", | |
"MatchesExpression": "matches the expression %s", |
plugins/Goals/lang/en.json
Outdated
"SingleGoalOverviewDocumentation": "This is an overview of the conversions for a single goal. %s The sparklines below the graph can be enlarged by clicking on them.", | ||
"ThereIsNoGoalToManage": "There is no goal to manage for website %s", | ||
"ThereIsNoGoalToManage": "There is no goal to manage for the %s website", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The placeholder contains the name of a website...
"ThereIsNoGoalToManage": "There is no goal to manage for the %s website", | |
"ThereIsNoGoalToManage": "There is no goal to manage for website %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense in German, but not in English.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be the reason why that feels wrong to me 🙈
Nevertheless we could consider adding quotes here as well. Wondering if it otherwise might look ugly in English if someone names his site with multiple words.
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
Co-authored-by: Stefan Giehl <stefan@matomo.org>
plugins/Goals/lang/en.json
Outdated
"GoalConversionsBy": "Goal %s conversions by type of visit", | ||
"GoalIsTriggered": "Goal is triggered", | ||
"GoalIsTriggeredWhen": "Goal is triggered when", | ||
"GoalConversionsBy": "%s-goal conversions by type of visit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work, as the goal name inserted in the placeholder might be anything. So might be better to quote it here as well.
"AddNewGoalOrEditExistingGoal": "%1$sAdd a new Goal%2$s or %3$sEdit%4$s existing Goals", | ||
"AllowGoalConvertedMoreThanOncePerVisit": "Allow Goal to be converted more than once per visit", | ||
"AddNewGoalOrEditExistingGoal": "%1$sAdd a new goal%2$s or %3$sEdit%4$s existing goals", | ||
"AllowGoalConvertedMoreThanOncePerVisit": "Allow goal to be converted more than once per visit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"AllowGoalConvertedMoreThanOncePerVisit": "Allow goal to be converted more than once per visit", | |
"AllowGoalConvertedMoreThanOncePerVisit": "Allow any goal to be converted more than once per visit", |
Can also possibly be "a", "the", "goals", or "this"(?)
Small improvement.
This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers |
"ColumnOrdersDocumentation": "The total number of Ecommerce orders which contained this %s at least once.", | ||
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the sum of Product quantities sold in all Ecommerce orders.", | ||
"ColumnOrdersDocumentation": "The total number of e-commerce orders which contained this %s at least once.", | ||
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the sum product quantity of all e-commerce orders sold.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the sum product quantity of all e-commerce orders sold.", | |
"ColumnPurchasedProductsDocumentation": "The number of purchased products is the combined product quantity of all e-commerce orders sold.", |
This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers |
Description:
Generally shorter and to the point.
In line with other edits.
Review