-
Notifications
You must be signed in to change notification settings - Fork 936
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
Fix typos in Private Tab #26316
Fix typos in Private Tab #26316
Conversation
@@ -1400,7 +1400,7 @@ Are you sure you want to do this? | |||
Browsing visibility | |||
</message> | |||
<message name="IDS_BRAVE_NEW_TAB_PRIVATE_VISIBILITY_DESCRIPTION" desc="Browsing visibility description shown when a user opens a private tab explaining private mode"> | |||
Note that -even in private windows- your browsing activity may still be visible to your ISP, network observers, or your employer (if you're using a work device/network). | |||
Note that - even in private windows - your browsing activity may still be visible to your ISP, network observers, or your employer (if you're using a work device/network). |
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.
I believe we should use a double dash as well in that place —
instead of -
, right @timchilds?
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's my understanding. cc our resident dash expert @rmcfadden3 to confirm.
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.
Official style is em dashes, no spaces. So:
Note that—even in private windows—your browsing activity…
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.
I've made the necessary adjustments to the text as requested. However, I would like to kindly mention that, although I understand the use of em dashes is grammatically correct in American English, as a non-native speaker, this style can be somewhat concerning. It has the potential to cause significant confusion for individuals whose primary language is not American English. Perhaps, a more universal and easily understood form could be achieved using commas instead.
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.
I agree with @simoarpe , we will definitely have confusions after translation to other languages.
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.
WDYT @AndyAnds265 ?
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.
hey all, sorry I missed this comment.
the localized side can be customized in the strings so unless something is hardcoded we should be able to edit by language
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.
At this point, wouldn't it be better to get rid of the dashes in favor of commas for all the languages?
Benefits:
- More consistent style across all the languages
- International users that keeps Brave in English (like myself) would find it easier to read. (In many languages dashes don't exist)
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.
Let's go with commas for the reasons @simoarpe highlighted.
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.
++ from string reviewers based on comment from @timchilds #26316 (comment)
Released in v1.75.18 |
Resolves brave/brave-browser#41848
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on