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

Lang plugin language reworked #510

Open
wants to merge 7 commits into
base: 4.x-dev
Choose a base branch
from

Conversation

comradekingu
Copy link
Contributor

Description:

Generally shorter and to the point.
In line with other edits.

Review

lang/en.json Outdated Show resolved Hide resolved
lang/en.json Outdated Show resolved Hide resolved
lang/en.json Outdated Show resolved Hide resolved
"PageViewTriggerDescription": "Triggered as soon as the Tag Manager is executed within the page.",
"PageLoadTimeTotalVariableName": "Total page-load time",
"PageOriginVariableDescription": "Returns a domain of the current URL with its protocol.",
"PageOriginVariableName": "Web-page origin",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This mess could be fixed by going "Website ___"?

lang/en.json Outdated Show resolved Hide resolved
"RandomNumberVariableName": "Random Number",
"ReferrerUrlVariableDescription": "Gets the value of the Referrer URL.",
"RandomNumberVariableName": "Random number",
"ReferrerUrlVariableDescription": "Gets the value of the referrer URL.",
"ReferrerUrlVariableName": "Referrer URL",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self, find out how this was quoted in the other PR.

lang/en.json Outdated
"SeoNumH1VariableName": "Number of H1 elements",
"SeoNumH2VariableDescription": "The number of H2 elements present on the current page. Useful for example for SEO Monitoring.",
"SeoNumH2VariableName": "Number of H2 elements",
"SettingCustomTemplatesDescription": "This setting defines who can use custom templates, and whether they should be turned off completely. Changing this setting can improve the security on your website. When you configure a trigger, tag, or a variable, some of them may allow a Matomo user to enter HTML or JavaScript which will be executed on your website. Entering custom code cannot only break the container in case there is an error, but also allows them to execute any JavaScript code on your website. This can be misused to steal for example sensitive information on your website. If you do not want to allow your team members to enter any JavaScript code, you may want to turn off this setting.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"SettingCustomTemplatesDescription": "This setting defines who can use custom templates, and whether they should be turned off completely. Changing this setting can improve the security on your website. When you configure a trigger, tag, or a variable, some of them may allow a Matomo user to enter HTML or JavaScript which will be executed on your website. Entering custom code cannot only break the container in case there is an error, but also allows them to execute any JavaScript code on your website. This can be misused to steal for example sensitive information on your website. If you do not want to allow your team members to enter any JavaScript code, you may want to turn off this setting.",
"SettingCustomTemplatesDescription": "This setting defines who can use custom templates, and whether they should be turned off completely. Changing this setting can improve the security on your website. When you configure a trigger, tag, or a variable, some of them may allow a Matomo user to enter HTML or JavaScript which will be executed on your website. Entering custom code cannot only break the container in case there is an error, but also allows them to execute any JavaScript code on your website. This can be misused to steal for example sensitive info on your website. If you do not want to allow your team members to enter any JavaScript code, you may want to turn off this setting.",

Pretty sure this string exists in reworked form somewhere.
Improvements possible.

lang/en.json Outdated Show resolved Hide resolved
lang/en.json Outdated
"DeleteTriggerConfirm": "Are you sure you want to delete this trigger? You cannot undo the deletion.",
"DeleteVariableConfirm": "Are you sure you want to delete this variable? You cannot undo the deletion.",
"DeleteVersionConfirm": "Are you sure you want to delete this version? You cannot undo the deletion.",
"DefaultValueHelp": "Configure a default value to use if the variable does not return a value. Please note an empty string ('') is considered to be a value and will not fall back to the default value, configure a lookup value for this case if needed. Also note that the default value will be applied before the lookup table is evaluated.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Here again we are missing the alert I feel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you want to alert the user to?

lang/en.json Outdated Show resolved Hide resolved
@comradekingu
Copy link
Contributor Author

comradekingu commented Nov 17, 2022

I don't know what to do here, so I piled on the new changes in a new commit.
Rebase to dev-5?
Hopefully this can reach translation, since that sits at 21% translated.
I think much of the reason why is because this is currently very inconsistent and cumbersome to deal with.

The "dataLayer" "Data-Layer" "data layer" situation is a bit confusing, and the titles are either title-case or not.
I think reviewing this for disastrous errors now and then revisiting with smaller changes in new PRs is the way to go.

lang/en.json Outdated Show resolved Hide resolved
lang/en.json Outdated Show resolved Hide resolved
@sgiehl
Copy link
Member

sgiehl commented Nov 21, 2022

@comradekingu Splitting this one into multiple PRs with fewer changes each, might make it a lot easier to review and merge it.

Co-authored-by: Stefan Giehl <[email protected]>
@comradekingu
Copy link
Contributor Author

comradekingu commented Nov 21, 2022

@sgiehl I am done changing it for now. Start at the top and make comments going down? 100 per day and some improvements and it is done this week.

I can keep one big file in my head and keep track of changes, but over multiple interconnected ones it becomes taxing to do.
It is also easier for me to go check my last entry in the commit log and know what sort of consistency that represented.

Right now there is a mix of sentence and title case, and "execute" "trigger" "run" and "fire" are all in use.
The bigger problem to solve is to get it all translatable. It may not be perfect, but as long as nothing is disastrously wrong we can get lots of translator eyes on it and all pull in the same direction.

lang/en.json Outdated Show resolved Hide resolved
"ZendeskChatTagName": "Zendesk Chat (formerly Zopim)",
"SettingElementVisibilityObserveDomChangesTitle": "Observe DOM Changes",
Copy link
Contributor

Choose a reason for hiding this comment

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

@comradekingu Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't really know, but adding it back in.

Copy link
Contributor

@AltamashShaikh AltamashShaikh left a comment

Choose a reason for hiding this comment

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

@comradekingu left few comments, I am trying to review this from bottom as there are lot of changes

"PreviewDebugEnabledNotificationLine1": "Preview mode is enabled%1$s. Access your website or enter a URL to debug the container now. ",
"PreviewDebugEnabledNotificationLine2": "If you want to %1$sshare the preview%2$s with someone else, please append %3$s or %4$s to the URL of your website.%5$sWhile this mode is enabled, the preview container will be automatically updated when you make a change.",
"PreviewDebugEnabledNotificationLine3": "Debug not working? Please check this %1$sFAQ%2$s.",
"PreviewDebugEnabledNotificationLine1": "Preview mode is on%1$s. Access your website or enter a URL to debug the container now. ",
Copy link
Contributor

Choose a reason for hiding this comment

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

enabled as earlier sounds more appropriate in comparison to on

Copy link
Contributor Author

@comradekingu comradekingu Nov 22, 2022

Choose a reason for hiding this comment

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

The idea is to never use enable/d or disable/d,
unless something can be on, and still disabled
when communicating that distinction is also meaningful.

https://hosted.weblate.org/languages/nb_NO/matomo/search/?offset=1&q=enabled&sort_by=-priority%2Cposition&checksum=
are the ones left (most of which are here)

"NoReleasesFound": "No release found.",
"NoReleasesFoundForContainer": "No release has been found for this container.",
"NoReleasesFoundForContainer": "Could not find any release in this container.",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the older text is more appropriate since release is for a container and not a sub part of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"NoReleasesFoundForContainer": "Could not find any release in this container.",
"NoReleasesFoundForContainer": "Could not find any releases in this container.",

"MatomoConfigurationMatomoJsEndpointDescription": "Allows setting the source path of the Matomo JavaScript tracker. If you are not using the \"Bundle tracker\" option.",
"MatomoConfigurationMatomoTrackingEndpointTitle": "Target path for tracking requests",
"MatomoConfigurationMatomoTrackingEndpointDescription": "Allows setting the target path for tracking requests.",
"MatomoTagDescription": "Matomo is the copylefted libre privacy-friendly open source analytics-platform.",
Copy link
Contributor

Choose a reason for hiding this comment

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

@sgiehl, @tsteur is this text okay as overall ?

"MatomoConfigurationMatomoAlwaysUseSendBeaconTitle": "Always use sendBeacon",
"MatomoConfigurationMatomoAlwaysUseSendBeaconDescription": "Enables sendBeacon usage instead of a regular ajax request. This means when a user clicks for example on an outlink, the navigation to this page will happen much faster.",
"MatomoConfigurationMatomoAlwaysUseSendBeaconDescription": "Turns on sendBeacon usage instead of a regular AJAX request. This means when a user clicks e.g. on an outlink, the navigation to this web page is much quicker.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Again "Enables" sounds more appropriate in the context

"MatomoConfigurationMatomoTrackVisibleContentImpressionsTitle": "Track visible content impressions",
"MatomoConfigurationMatomoTrackVisibleContentImpressionsDescription": "Tracks content by scanning the entire DOM for all content blocks, but only tracks content impressions once the user scrolls to the content and the content is actually visible.",
"MatomoConfigurationMatomoDisableCookiesTitle": "No cookies",
"MatomoConfigurationMatomoDisableCookiesDescription": "Turns off all first-party cookies.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Again Disable vs Off

"DriftHelp": "This tag allows you to add the Drift contact form to your website.",
"AllDownloadsClickTriggerDescription": "Triggered when a link is clicked which links to a downloadable file. It will be triggered on left, middle and right click.",
"DriftHelp": "This tag allows adding the drift contact form to your website.",
"AllDownloadsClickTriggerDescription": "Triggered when a link linking to a downloadable file is left, middle, or right clicked.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks too confusing

"DeleteVersionConfirm": "Are you sure you want to delete this version? You cannot undo the deletion.",
"DefaultValueHelp": "Set a default value to use if the variable does not return a value. Please note an empty string ('') is consideree a value and will not fall back to the default value, configure a lookup value if needed in this case. Also note that the default value will be applied before the lookup table is evaluated.",
"DeleteContainerConfirm": "Delete this container for good? Please also make sure to remove all embedded code snippets for this container from your website as the container files will no longer be available after you have deleted this container.",
"DeleteTagConfirm": "Delete this tag for good?",
Copy link
Contributor

Choose a reason for hiding this comment

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

@tsteur Does this sound okay to you ?
For me it doesn't alert the user as it should I feel

"DriftHelp": "This tag allows you to add the Drift contact form to your website.",
"AllDownloadsClickTriggerDescription": "Triggered when a link is clicked which links to a downloadable file. It will be triggered on left, middle and right click.",
"DriftHelp": "This tag allows adding the drift contact form to your website.",
"AllDownloadsClickTriggerDescription": "Triggered when a link linking to a downloadable file is left, middle, or right clicked.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"AllDownloadsClickTriggerDescription": "Triggered when a link linking to a downloadable file is left, middle, or right clicked.",
"AllDownloadsClickTriggerDescription": "Triggered when a link to a downloadable file is left-, middle-, or right-clicked.",

"CustomHtmlTagHelpText": "%1$sLearn more%2$s",
"CustomHtmlHtmlPositionTitle": "Position",
"CustomHtmlHtmlPositionDescription": "Define the position of where the HTML should be inserted into your website.",
"CustomHtmlHtmlPositionDescription": "Define the position of where to instart the HTML into your website.",
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a typo for insert ?

"DeleteVersionConfirm": "Are you sure you want to delete this version? You cannot undo the deletion.",
"DefaultValueHelp": "Set a default value to use if the variable does not return a value. Please note an empty string ('') is consideree a value and will not fall back to the default value, configure a lookup value if needed in this case. Also note that the default value will be applied before the lookup table is evaluated.",
"DeleteContainerConfirm": "Delete this container for good? Please also make sure to remove all embedded code snippets for this container from your website as the container files will no longer be available after you have deleted this container.",
"DeleteTagConfirm": "Delete this tag for good?",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"DeleteTagConfirm": "Delete this tag for good?",
"DeleteTagConfirm": "Delete this tag permanently?",

(I would go without "permanently", but I understand its importance)
Leaving this open, as there are plenty "for good" strings.

"CustomHtmlTagHelpText": "%1$sLearn more%2$s",
"CustomHtmlHtmlPositionTitle": "Position",
"CustomHtmlHtmlPositionDescription": "Define the position of where the HTML should be inserted into your website.",
"CustomHtmlHtmlPositionDescription": "Define the position of where to instart the HTML into your website.",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"CustomHtmlHtmlPositionDescription": "Define the position of where to instart the HTML into your website.",
"CustomHtmlHtmlPositionDescription": "Define the position of where to insert the HTML into your website.",

Good catch. Was thinking of two things, and the resolved discussion of why it isn't "onto your website".

@AltamashShaikh
Copy link
Contributor

@comradekingu left my comments and have asked for other team members input too on this, btw thanks for the translation work it was alot 👍

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.

3 participants