From 889757e1c94433594112efac5a4baf6f2f3026df Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Sat, 8 Oct 2016 12:02:29 -0700 Subject: [PATCH] Updated General Preferences - Show home button now disables itself when home page is cleared (null/empty) - Removed appearance settings; moved to under "My home page is" per the mockup on https://github.com/brave/browser-laptop/issues/2108 - Bookmarks toolbar switches were changed into a single drop down Unit tests are in place and working + I've manually at least two of the scenarios outlined in the attached steps Fixes https://github.com/brave/browser-laptop/issues/2108 Fixes https://github.com/brave/browser-laptop/issues/3008 Fixes https://github.com/brave/browser-laptop/issues/2111 Possibly fixes https://github.com/brave/browser-laptop/issues/3544 Auditors: @BrendanEich - you had previously reviewed the settings code; LMK how the changes in `./js/settings.js` look @bbondy - small changes EXCEPT the new config value for BTB. This only had two touchpoints (main.js, contextMenus.js) versus the password manager changes Tweaks made per review w/ Brad. Includes: - reducing space between label and field in preferences - the lone toggle switch was pulled over to the left - import button is moved up - import button area is no longer a separate grouping Upgrade test plan: 1. Install a previous version of Brave (0.12.4 or older); ENABLE favicons and ENABLE show only favicons 2. Upgrade to this version (0.12.5?). Confirm it consolidates preference to "Favicons only" in preferences drop down 3. Install a previous version of Brave (0.12.4 or older); ENABLE favicons but DISABLE show only favicons 4. Upgrade to this version (0.12.5?). Confirm it consolidates preference to "Text and Favicons" in preferences drop down 5. Install a previous version of Brave (0.12.4 or older); DISABLE favicons and DISABLE show only favicons 6. Upgrade to this version (0.12.5?). Confirm it consolidates preference to "Text only" in preferences drop down 7. Uninstall Brave completely; move session file out of the way Regular test plan: ** consolidation of bookmarks toolbar settings** 1. Install a clean version of Brave (0.12.5?) 2. Launch preferences, confirm preference defaults to "Text only" 3. Confirm no favicons show (only text): - at the root level of the bookmarks toolbar - when expanding a folder inside the bookmarks toolbar 4. Repeat step 2/3 for the other two settings. NOTE: show only favicon does not apply to items inside bookmark folders ** bookmark toolbar enable/disable change ** 5. Go to preferences and toggle "Always show the bookmarks toolbar" 6. If disabled, notice how drop down is now disabled too (not allowing you to change) 7. When enabled, notice how drop down is re-enabled ** disabling of show home button ** 8. Set your home page to brave.com and enable "show home button" 9. Erase out the value in home page and confirm "show home button" is: - is turned off - disabled completely (not clickable) 10. Type a non-whitespace character or characters into home page box 11. Confirm the "show home button" is clickable again ** overall general settings tab ** 12. Ensure screen matches corresponding parts of mockup shown in https://github.com/brave/browser-laptop/issues/2108 13. Ensure screen matches corresponding parts of mockup shown in https://github.com/brave/browser-laptop/issues/2111 --- app/common/constants/bookmarksToolbarMode.js | 11 ++ .../locales/en-US/preferences.properties | 6 +- js/about/aboutActions.js | 2 +- js/about/preferences.js | 71 ++++++++--- js/components/main.js | 6 +- js/constants/settings.js | 29 +++-- js/contextMenus.js | 4 +- js/settings.js | 41 ++++-- less/about/preferences.less | 11 +- test/unit/settingsTest.js | 117 +++++++++++------- 10 files changed, 200 insertions(+), 98 deletions(-) create mode 100644 app/common/constants/bookmarksToolbarMode.js diff --git a/app/common/constants/bookmarksToolbarMode.js b/app/common/constants/bookmarksToolbarMode.js new file mode 100644 index 00000000000..824cbfb7283 --- /dev/null +++ b/app/common/constants/bookmarksToolbarMode.js @@ -0,0 +1,11 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const settings = { + TEXT_ONLY: 'textOnly', + TEXT_AND_FAVICONS: 'textAndFavicons', + FAVICONS_ONLY: 'faviconsOnly' +} + +module.exports = settings diff --git a/app/extensions/brave/locales/en-US/preferences.properties b/app/extensions/brave/locales/en-US/preferences.properties index a8b39adf7d6..6bca8a185a8 100644 --- a/app/extensions/brave/locales/en-US/preferences.properties +++ b/app/extensions/brave/locales/en-US/preferences.properties @@ -153,7 +153,7 @@ managePasswords=Manage passwords… sitePermissions=Saved Site Permissions sitePermissionsExceptions=Saved Site Exceptions selectedLanguage=Language: -bookmarkToolbarSettings=Bookmarks Bar Settings +bookmarkToolbarSettings=Bookmarks Bar bookmarkToolbar=Always show the bookmarks bar bookmarkToolbarShowFavicon=Favicons bookmarkToolbarShowOnlyFavicon=Show only favicon @@ -220,7 +220,6 @@ allowUntilRestart=Allow until restart flashAllowAlways=Allow until {{time}} alwaysAllow=Always allow alwaysDeny=Always deny -appearanceSettings=Appearance Settings autoHideMenuBar=Hide the menu bar by default disableTitleMode=Always show the URL bar tabsSettings=Tabs Settings @@ -247,3 +246,6 @@ importNow=Import now… clearAll=Clear all sendCrashReports=Send anonymous crash reports to Brave (requires browser restart) sendUsageStatistics=Automatically send usage statistics to Brave +bookmarksBarTextOnly=Text only +bookmarksBarTextAndFavicon=Text and Favicons +bookmarksBarFaviconOnly=Favicons only diff --git a/js/about/aboutActions.js b/js/about/aboutActions.js index d2a818c863b..5702f55b1c4 100644 --- a/js/about/aboutActions.js +++ b/js/about/aboutActions.js @@ -210,7 +210,7 @@ const aboutActions = { ipc.sendToHost(messages.CLEAR_BROWSING_DATA_NOW, clearBrowsingDataDetail) }, - importBrowerDataNow: function () { + importBrowserDataNow: function () { ipc.send(messages.IMPORT_BROWSER_DATA_NOW) }, diff --git a/js/about/preferences.js b/js/about/preferences.js index 53318929169..daab6c2f1f9 100644 --- a/js/about/preferences.js +++ b/js/about/preferences.js @@ -20,6 +20,7 @@ const messages = require('../constants/messages') const settings = require('../constants/settings') const coinbaseCountries = require('../constants/coinbaseCountries') const {passwordManagers, extensionIds} = require('../constants/passwordManagers') +const bookmarksToolbarMode = require('../../app/common/constants/bookmarksToolbarMode') const aboutActions = require('./aboutActions') const getSetting = require('../settings').getSetting const SortableTable = require('../components/sortableTable') @@ -138,7 +139,14 @@ class SettingCheckbox extends ImmutableComponent { } render () { - return
+ const props = { + style: this.props.style, + className: 'settingItem' + } + if (this.props.id) { + props.id = this.props.id + } + return
) }) + const homepageValue = getSetting(settings.HOMEPAGE, this.props.settings) + const homepage = homepageValue && homepageValue.trim() + const disableShowHomeButton = !homepage || !homepage.length + const disableBookmarksBarSelect = !getSetting(settings.SHOW_BOOKMARKS_TOOLBAR, this.props.settings) const defaultLanguage = this.props.languageCodes.find((lang) => lang.includes(navigator.language)) || 'en-US' + return
@@ -604,7 +629,26 @@ class GeneralTab extends ImmutableComponent { + onChange={changeSetting.bind(null, this.onChangeSetting, settings.HOMEPAGE)} /> + + + { + isDarwin ? null : + } + + + +