From c08cee0facd3daaf83103155ee82f1fd43ec58e9 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Thu, 29 Sep 2016 16:13:28 -0700 Subject: [PATCH] Update `menu.js` to also rebuild when handling (add/remove) bookmark folders Added tests which ensure menu gets rebuild when adding bookmark or bookmark folder Fixes https://github.com/brave/browser-laptop/issues/4227 Fixes https://github.com/brave/browser-laptop/issues/3968 Auditors: @bridiver Test Plan: 1) Launch Brave and ensure bookmarks toolbar is showing 2) Right click toolbar, choose Add Folder 3) Specify the name "1234" and hit save 4) Ensure "Bookmarks" menu is updated after saving 5) Save a bookmark inside of this folder 6) Verify "Bookmarks" menu looks correct Auditors: @bridiver --- app/browser/menu.js | 4 +- docs/state.md | 4 +- test/components/bookmarksTest.js | 233 ++++++++++++++++++++----------- 3 files changed, 152 insertions(+), 89 deletions(-) diff --git a/app/browser/menu.js b/app/browser/menu.js index 060d7291466..ba736747955 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -616,14 +616,14 @@ const doAction = (action) => { }) break case appConstants.APP_ADD_SITE: - if (action.tag === siteTags.BOOKMARK) { + if (action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) { appDispatcher.waitFor([appStore.dispatchToken], () => { createMenu() }) } break case appConstants.APP_REMOVE_SITE: - if (action.tag === siteTags.BOOKMARK) { + if (action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) { appDispatcher.waitFor([appStore.dispatchToken], () => { createMenu() }) diff --git a/docs/state.md b/docs/state.md index db6cc0208bb..6ba97fc5ae6 100644 --- a/docs/state.md +++ b/docs/state.md @@ -218,8 +218,8 @@ AppStore gridLayout: string // 'small', 'medium', 'large' } }, - menubar: { - template: object // windows only: template object with Menubar control + menu: { + template: object // used on Windows and by our tests: template object with Menubar control } } ``` diff --git a/test/components/bookmarksTest.js b/test/components/bookmarksTest.js index 5c8221f3c16..456fa532f02 100644 --- a/test/components/bookmarksTest.js +++ b/test/components/bookmarksTest.js @@ -2,8 +2,9 @@ const Brave = require('../lib/brave') const {urlInput, navigator, navigatorNotBookmarked, saveButton, deleteButton} = require('../lib/selectors') +const siteTags = require('../../js/constants/siteTags') -describe('bookmarks', function () { +describe('bookmark tests', function () { function * setup (client) { yield client .waitUntilWindowLoaded() @@ -13,125 +14,187 @@ describe('bookmarks', function () { .waitForEnabled(urlInput) } - describe('with title', function () { - Brave.beforeAll(this) + describe('bookmarks', function () { + describe('pages with title', function () { + Brave.beforeAll(this) - before(function * () { - this.page1Url = Brave.server.url('page1.html') + before(function * () { + this.page1Url = Brave.server.url('page1.html') - yield setup(this.app.client) + yield setup(this.app.client) - yield this.app.client - .waitForUrl(Brave.newTabUrl) - .loadUrl(this.page1Url) - .windowParentByUrl(this.page1Url) - .waitForVisible(navigator) - .moveToObject(navigator) - .waitForVisible(navigatorNotBookmarked) - .click(navigatorNotBookmarked) - .waitForVisible(saveButton) - }) + yield this.app.client + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowParentByUrl(this.page1Url) + .waitForVisible(navigator) + .moveToObject(navigator) + .waitForVisible(navigatorNotBookmarked) + .click(navigatorNotBookmarked) + .waitForVisible(saveButton) + }) - it('fills in the title field', function * () { - yield this.app.client - .waitForExist('#bookmarkName input') - .getValue('#bookmarkName input').should.eventually.be.equal('Page 1') - }) + it('fills in the title field', function * () { + yield this.app.client + .waitForExist('#bookmarkName input') + .getValue('#bookmarkName input').should.eventually.be.equal('Page 1') + }) - it('fills in the url field', function * () { - yield this.app.client - .waitForExist('#bookmarkLocation input') - .getValue('#bookmarkLocation input').should.eventually.be.equal(this.page1Url) + it('fills in the url field', function * () { + yield this.app.client + .waitForExist('#bookmarkLocation input') + .getValue('#bookmarkLocation input').should.eventually.be.equal(this.page1Url) + }) + + describe('saved with a title', function () { + before(function * () { + yield this.app.client + .click(saveButton) + }) + + it('displays title', function * () { + yield this.app.client + .waitUntil(function () { + return this.getText('.bookmarkText') + .then((val) => val === 'Page 1') + }) + }) + + describe('and then removed', function () { + before(function * () { + yield this.app.client + .waitForVisible(navigator) + .moveToObject(navigator) + .waitForVisible(navigatorNotBookmarked) + .click(navigatorNotBookmarked) + .waitForVisible(deleteButton) + .click(deleteButton) + }) + it('removes the bookmark from the toolbar', function * () { + yield this.app.client + .waitForExist('.bookmarkText', 1000, true) + }) + }) + }) }) - describe('saved with a title', function () { + describe('pages without title', function () { + Brave.beforeAll(this) + before(function * () { + this.page1Url = Brave.server.url('page_no_title.html') + + yield setup(this.app.client) + yield this.app.client - .click(saveButton) + .waitForUrl(Brave.newTabUrl) + .loadUrl(this.page1Url) + .windowParentByUrl(this.page1Url) + .moveToObject(navigator) + .waitForExist(navigatorNotBookmarked) + .moveToObject(navigator) + .click(navigatorNotBookmarked) + .waitForVisible(saveButton + ':not([disabled]') }) - it('displays title', function * () { + it('leaves the title field blank', function * () { yield this.app.client - .waitUntil(function () { - return this.getText('.bookmarkText') - .then((val) => val === 'Page 1') - }) + .waitForExist('#bookmarkName input') + .getValue('#bookmarkName input').should.eventually.be.equal('') }) - describe('and then removed', function () { + it('fills in the url field', function * () { + yield this.app.client + .waitForExist('#bookmarkLocation input') + .getValue('#bookmarkLocation input').should.eventually.be.equal(this.page1Url) + }) + + describe('saved without a title', function () { before(function * () { yield this.app.client - .waitForVisible(navigator) - .moveToObject(navigator) - .waitForVisible(navigatorNotBookmarked) - .click(navigatorNotBookmarked) - .waitForVisible(deleteButton) - .click(deleteButton) + .click(saveButton) }) - it('removes the bookmark from the toolbar', function * () { + it('displays URL', function * () { + const page1Url = this.page1Url yield this.app.client - .waitForExist('.bookmarkText', 1000, true) + .waitUntil(function () { + return this.getText('.bookmarkText') + .then((val) => val === page1Url) + }) + }) + describe('and then removed', function () { + before(function * () { + yield this.app.client + .click(navigatorNotBookmarked) + .waitForExist(deleteButton) + .click(deleteButton) + }) + it('removes the bookmark from the toolbar', function * () { + yield this.app.client + .waitForExist('.bookmarkText', 1000, true) + }) }) }) }) }) - describe('without title', function () { + describe('menu behavior', function () { Brave.beforeAll(this) before(function * () { - this.page1Url = Brave.server.url('page_no_title.html') - yield setup(this.app.client) - - yield this.app.client - .waitForUrl(Brave.newTabUrl) - .loadUrl(this.page1Url) - .windowParentByUrl(this.page1Url) - .moveToObject(navigator) - .waitForExist(navigatorNotBookmarked) - .moveToObject(navigator) - .click(navigatorNotBookmarked) - .waitForVisible(saveButton + ':not([disabled]') }) - it('leaves the title field blank', function * () { - yield this.app.client - .waitForExist('#bookmarkName input') - .getValue('#bookmarkName input').should.eventually.be.equal('') - }) + it('rebuilds the menu when a folder is added', function * () { + const folderName = 'bookmark-folder-rebuild-menu-demo' - it('fills in the url field', function * () { yield this.app.client - .waitForExist('#bookmarkLocation input') - .getValue('#bookmarkLocation input').should.eventually.be.equal(this.page1Url) + .addSite({ + customTitle: folderName, + folderId: 1, + parentFolderId: 0, + tags: [siteTags.BOOKMARK_FOLDER] + }, siteTags.BOOKMARK_FOLDER) + .waitUntil(function () { + return this.getAppState().then((val) => { + const bookmarksMenu = val.value.menu.template.find((item) => { + return item.label === 'Bookmarks' + }) + if (bookmarksMenu && bookmarksMenu.submenu) { + const bookmarkFolder = bookmarksMenu.submenu.find((item) => { + return item.label === folderName + }) + if (bookmarkFolder) return true + } + return false + }) + }) }) - describe('saved without a title', function () { - before(function * () { - yield this.app.client - .click(saveButton) - }) - it('displays URL', function * () { - const page1Url = this.page1Url - yield this.app.client - .waitUntil(function () { - return this.getText('.bookmarkText') - .then((val) => val === page1Url) + it('rebuilds the menu when a bookmark is added', function * () { + const bookmarkTitle = 'bookmark-rebuild-menu-demo' + + yield this.app.client + .addSite({ + lastAccessedTime: 456, + tags: [siteTags.BOOKMARK], + location: 'https://brave.com', + title: bookmarkTitle + }, siteTags.BOOKMARK) + .waitUntil(function () { + return this.getAppState().then((val) => { + const bookmarksMenu = val.value.menu.template.find((item) => { + return item.label === 'Bookmarks' + }) + if (bookmarksMenu && bookmarksMenu.submenu) { + const bookmark = bookmarksMenu.submenu.find((item) => { + return item.label === bookmarkTitle + }) + if (bookmark) return true + } + return false }) - }) - describe('and then removed', function () { - before(function * () { - yield this.app.client - .click(navigatorNotBookmarked) - .waitForExist(deleteButton) - .click(deleteButton) }) - it('removes the bookmark from the toolbar', function * () { - yield this.app.client - .waitForExist('.bookmarkText', 1000, true) - }) - }) }) }) })