Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Changed share l10n string to use a generic template for each share menu items. #12445

Merged
merged 2 commits into from
Jan 2, 2018
Merged

Changed share l10n string to use a generic template for each share menu items. #12445

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
189b7d5
Changed share l10n string to use a generic template for each share me…
Dec 30, 2017
6e559e7
Adds replacements to the transaltion function
NejcZdovc Dec 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions app/browser/menu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,13 @@ const createFileSubmenu = () => {
submenu: [
CommonMenu.simpleShareActiveTabMenuItem('emailPageLink', 'email'),
CommonMenu.separatorMenuItem,
CommonMenu.simpleShareActiveTabMenuItem('tweetPageLink', 'twitter'),
CommonMenu.simpleShareActiveTabMenuItem('facebookPageLink', 'facebook'),
CommonMenu.simpleShareActiveTabMenuItem('pinterestPageLink', 'pinterest'),
CommonMenu.simpleShareActiveTabMenuItem('googlePlusPageLink', 'googlePlus'),
CommonMenu.simpleShareActiveTabMenuItem('linkedInPageLink', 'linkedIn'),
CommonMenu.simpleShareActiveTabMenuItem('bufferPageLink', 'buffer'),
CommonMenu.simpleShareActiveTabMenuItem('redditPageLink', 'reddit')
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'twitter'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'facebook'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'pinterest'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'googlePlus'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'linkedIn'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'buffer'),
CommonMenu.simpleShareActiveTabMenuItem('sharePageLink', 'reddit')
]
},
// Move inside share menu when it's enabled
Expand Down
4 changes: 3 additions & 1 deletion app/common/commonMenu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,10 @@ module.exports.printMenuItem = () => {
}

module.exports.simpleShareActiveTabMenuItem = (l10nId, type, accelerator) => {
const siteName = type.charAt(0).toUpperCase() + type.slice(1)

return {
label: locale.translation(l10nId),
label: locale.translation(l10nId).replace(/{{\s*siteName\s*}}/, siteName),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change this one to locale.translation(l10nId, {siteName: siteName})? We already have a mechanism for variables in the translations.

Copy link
Author

@sergio-rojasa sergio-rojasa Dec 30, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I tried your suggestion and I tried other way using ${locale.translation(l10nId)} ${siteName} but it also didn't work. when I lookup locale.translation, I found out that it only accepted one argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure that I follow, this change is not working for you?

Copy link
Author

Choose a reason for hiding this comment

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

The change is not working for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, let me try it

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I see what's going on, will fix it, so that this will work as well

Copy link
Contributor

Choose a reason for hiding this comment

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

@sergiorojasa ok this should be working now, can you please check out if everything is ok now? I pushed my fix commit to your PR

accelerator,
click: function (item, focusedWindow) {
appActions.simpleShareActiveTabRequested(type)
Expand Down
10 changes: 2 additions & 8 deletions app/extensions/brave/locales/en-US/menu.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ braveryPayments=Brave Payments…
braverySite=Site Shield Settings…
braveryStartUsingPayments=Start using Brave Payments…
bringAllToFront=Bring All to Front
bufferPageLink=Share on Buffer…
checkForUpdates=Check for Updates…
cleanReload=Clean Reload
clearBrowsingData=Clear Browsing Data…
Expand Down Expand Up @@ -72,15 +71,13 @@ editFolder=Edit Folder…
emailPageLink=Email Page Link…
exportBookmarks=Export Bookmarks…
extensionsManager=Extensions…
facebookPageLink=Share on Facebook…
file=File
find=Find…
findNext=Find Next
findOnPage=Find on Page…
findPrevious=Find Previous
forgetLearnedSpelling=Forget Learned Spelling
forward=Forward
googlePlusPageLink=Share on Google+…
help=Help
hideBrave=Hide Brave
hideOthers=Hide Others
Expand All @@ -92,7 +89,6 @@ importFrom=Import from…
inspectElement=Inspect Element
learnSpelling=Learn Spelling
licenseText=This software uses libraries from the FFmpeg project under the LGPLv2.1
linkedInPageLink=Share on LinkedIn…
lookupSelection=Look Up “{{selectedVariable}}”
mergeAllWindows=Merge All Windows
minimize=Minimize
Expand Down Expand Up @@ -124,15 +120,13 @@ pasteAndGo=Paste and Go
pasteAndSearch=Paste and Search
pasteWithoutFormatting=Paste Without Formatting
pinTab=Pin
pinterestPageLink=Share on Pinterest…
preferences=Preferences…
print=Print…
quit=Quit
quitApp=Quit Brave
readingView=Reading View
recentlyClosed=Recently Closed
recentlyVisited=Recently Visited
redditPageLink=Share on Reddit…
redo=Redo
reloadPage=Reload Page
reloadTab=Reload
Expand All @@ -149,6 +143,7 @@ selectPreviousTab=Select Previous Tab
services=Services
settings=Settings…
share=Share…
sharePageLink=Share on {{siteName}}...
showAll=Show All
showAllHistory=Show History
stop=Stop
Expand All @@ -159,7 +154,6 @@ toggleBrowserConsole=Toggle Browser Console
toggleDeveloperTools=Toggle Developer Tools
toggleFullScreenView=Toggle Full Screen View
toolbars=Toolbars
tweetPageLink=Tweet Page…
undo=Undo
unmuteTab=Unmute Tab
unmuteTabs=Unmute Tabs
Expand All @@ -172,4 +166,4 @@ window=Window
zoom=Zoom
zoom=Zoom
zoomIn=Zoom In
zoomOut=Zoom Out
zoomOut=Zoom Out
8 changes: 1 addition & 7 deletions app/locale.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,7 @@ var rendererIdentifiers = function () {
'reopenLastClosedTab',
'print',
'emailPageLink',
'tweetPageLink',
'facebookPageLink',
'pinterestPageLink',
'googlePlusPageLink',
'linkedInPageLink',
'bufferPageLink',
'redditPageLink',
'sharePageLink',
'findOnPage',
'find',
'checkForUpdates',
Expand Down
2 changes: 1 addition & 1 deletion test/unit/app/browser/shareTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('share API', function () {
assert.equal(this.mockTabs.create.withArgs(expectedUrl).callCount, 0)
})
})
describe('other', function () {
describe('other share page', function () {
it('calls createTabRequested with correct args', function () {
Object.entries(share.templateUrls).forEach(([shareType, template]) => {
if (shareType === 'email') {
Expand Down
16 changes: 8 additions & 8 deletions test/unit/app/common/commonMenuTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,14 +105,14 @@ describe('Common menu module unit tests', function () {
describe('simpleShareActiveTabMenuItem', function () {
it('has the expected defaults set', function () {
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'emailPageLink', 'email', 'CmdOrCtrl+Shift+I'), true)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'tweetPageLink', 'twitter'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'facebookPageLink', 'facebook'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'pinterestPageLink', 'pinterest'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'googlePlusPageLink', 'googlePlus'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'linkedInPageLink', 'linkedIn'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'bufferPageLink', 'buffer'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'redditPageLink', 'reddit'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'diggPageLink', 'digg'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'twitter'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'facebook'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'pinterest'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'googlePlus'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'linkedIn'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'buffer'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'reddit'), false)
checkExpectedDefaults(commonMenu.simpleShareActiveTabMenuItem.bind(null, 'sharePageLink', 'digg'), false)
})
})

Expand Down