Merge pull request #199 from matatk/sidebar-refinements

Sidebar refinements

.eslintignore

@@ -1,2 +1,3 @@
+build/
+_site/
 test/test-code-in-harness-*.js
-_site

README.md

@@ -209,7 +209,7 @@ Changes
 
 -   2.4.0 - in development
     -   Show the current keyboard commands on the splash page and allow the user to update them on Chrome and Opera. \[[\#187](https://github.com/matatk/landmarks/pull/187)\]
-    -   Offer an optional sidebar instead of the pop-up on Firefox and Opera. \[[\#188](https://github.com/matatk/landmarks/pull/188)\]
+    -   Offer an optional sidebar instead of the pop-up on Firefox and Opera. \[[\#188](https://github.com/matatk/landmarks/pull/188), [\#199](https://github.com/matatk/landmarks/pull/199)\]
 	-   Massive re-organisation of the code to make it easier to manage and accommodate and take advantage of cross-browser differences. \[[\#191](https://github.com/matatk/landmarks/pull/191)\]
 -   2.3.1 - 9th of June 2018
     -   Support multiple labelling elements when `aria-labelledby` is used. \[[\#176](https://github.com/matatk/landmarks/pull/176)\]

package.json

@@ -21,9 +21,11 @@
     "precommit": "npm run build:all",
     "profile": "node scripts/profile.js",
     "start:firefox": "web-ext run --firefox=firefoxdeveloperedition --browser-console --source-dir build/firefox/",
-    "pretest": "eslint . && rollup src/code/landmarksFinder.js -f cjs > test/test-code-in-harness-landmarks.js && rollup src/code/contrastChecker.js -f cjs > test/test-code-in-harness-contrast.js",
+    "pretest": "eslint .",
     "test": "npm run test:landmarks && npm run test:contrast",
+    "pretest:contrast": "rollup src/code/contrastChecker.js -f cjs > test/test-code-in-harness-contrast.js",
     "test:contrast": "node test/test-contrast.js",
+    "pretest:landmarks": "rollup src/code/landmarksFinder.js -f cjs > test/test-code-in-harness-landmarks.js",
     "test:landmarks": "node test/test-landmarks.js"
   },
   "devDependencies": {

scripts/build.js

@@ -77,6 +77,26 @@ const linters = Object.freeze({
 let testMode = false	// are we building a test (alpha/beta) version?
 
 
+function doReplace(files, from, to, message) {
+	try {
+		const changes = replace.sync({
+			'files': files,
+			'from': from,
+			'to': to
+		})
+		ok(message, changes.join(', '))
+	} catch (err) {
+		error('Error occurred:', err)
+	}
+}
+
+
+function ok() {
+	const argStrings = [...arguments].map(x => String(x))
+	console.error(chalk.green.apply(this, ['✔'].concat(argStrings)))
+}
+
+
 function error() {
 	const argStrings = [...arguments].map(x => String(x))
 	console.error(chalk.bold.red.apply(this, ['✖'].concat(argStrings)))
@@ -122,37 +142,64 @@ function pathToBuild(browser) {
 
 
 async function flattenCode(browser) {
-	const ioPairs = [{
+	logStep('Flattening JavaScript code...')
+
+	const ioPairsAndGlobals = [{
 		input: path.join(srcCodeDir, '_background.js'),
-		output: path.join(pathToBuild(browser), 'background.js'),
+		output: path.join(pathToBuild(browser), 'background.js')
 	}, {
 		input: path.join(srcCodeDir, '_content.js'),
-		output: path.join(pathToBuild(browser), 'content.js'),
+		output: path.join(pathToBuild(browser), 'content.js')
 	}, {
 		input: path.join(srcCodeDir, '_options.js'),
-		output: path.join(pathToBuild(browser), 'options.js'),
+		output: path.join(pathToBuild(browser), 'options.js')
 	}, {
 		input: path.join(srcCodeDir, '_popup.js'),
 		output: path.join(pathToBuild(browser), 'popup.js'),
+		globals: {
+			INTERFACE: 'popup'
+		}
 	}]
 
+	// The sidebar panel is 'forked' from the popup HTML file. The JavaScript
+	// differs slightly. It uses popup.css.
+	if (browser === 'firefox' || browser === 'opera') {
+		ioPairsAndGlobals.push({
+			input: path.join(srcCodeDir, '_popup.js'),
+			output: path.join(pathToBuild(browser), 'panel.js'),
+			globals: {
+				INTERFACE: 'sidebar'
+			}
+		})
+	}
+
+	// Now create an array of full bundle options to pass to rollup. Each
+	// element of these specifies all the common rollup and terser options.
+
 	const bundleOptions = []
 
-	for (const ioPair of ioPairs) {
+	for (const ioPair of ioPairsAndGlobals) {
 		const bundleOption = {}
+
+		const defines = {
+			BROWSER: browser
+		}
+
+		for (const global in ioPair.globals) {
+			defines[global] = ioPair.globals[global]
+		}
+
 		bundleOption.input = {
 			input: ioPair.input,
 			plugins: [terser({
 				mangle: false,
 				compress: {
 					defaults: false,
-					global_defs: {       // eslint-disable-line camelcase
-						BROWSER: browser
-					},
+					global_defs: defines,  // eslint-disable-line camelcase
 					conditionals: true,
-					dead_code: true,     // eslint-disable-line camelcase
+					dead_code: true,       // eslint-disable-line camelcase
 					evaluate: true,
-					side_effects: true,  // eslint-disable-line camelcase
+					side_effects: true,    // eslint-disable-line camelcase
 					switches: true,
 					unused: true,
 					passes: 2  // stops stray "firefox" string in compatibility
@@ -166,43 +213,55 @@ async function flattenCode(browser) {
 			}),
 			esformatter()]
 		}
+
 		bundleOption.output = {
 			file: ioPair.output,
 			format: 'iife'
 		}
+
 		bundleOptions.push(bundleOption)
 	}
 
+	// Run each bundle through rollup, terser and esformatter.
+
 	for (const options of bundleOptions) {
 		const bundle = await rollup.rollup(options.input)
 		await bundle.write(options.output)
 	}
-
-	console.log(chalk.green(`✔ flattened code written for ${browser}.`))
 }
 
 
 function copyStaticFiles(browser) {
 	logStep('Copying static files...')
 	fse.copySync(srcStaticDir, pathToBuild(browser))
 
-	function doReplace(from, to, message) {
-		try {
-			const changes = replace.sync({
-				'files': path.join(pathToBuild(browser), '*.html'),
-				'from': from,
-				'to': to
-			})
-			console.log(message, changes.join(', '))
-		} catch (err) {
-			error('Error occurred:', err)
-		}
-	}
-
 	if (browser === 'chrome' || browser === 'edge') {
-		doReplace(/<!-- ui -->[\s\S]*<!-- \/ui -->\s*/,
+		doReplace(
+			path.join(pathToBuild(browser), '*.html'),
+			/<!-- ui -->[\s\S]*?<!-- \/ui -->\s*/g,
 			'',
 			'Removed UI options in:')
+		doReplace(
+			path.join(pathToBuild(browser), '*.css'),
+			/\/\* ui \*\/[\s\S]*?\/\* \/ui \*\/\s*/g,
+			'',
+			'Removed UI styles in:')
+	}
+
+	// The sidebar panel is 'forked' from the popup HTML file. The JavaScript
+	// differs slightly. It uses popup.css.
+	if (browser === 'firefox' || browser === 'opera') {
+		fs.copyFileSync(
+			path.join(srcStaticDir, 'popup.html'),
+			path.join(pathToBuild(browser), 'panel.html'))
+
+		ok(`also created panel.html for ${browser}.`)
+
+		doReplace(
+			path.join(pathToBuild(browser), 'panel.html'),
+			'popup.js',
+			'panel.js',
+			'Included sidebar code in:')
 	}
 }
 
@@ -228,7 +287,7 @@ function mergeMessages(browser) {
 		fs.writeFileSync(destinationFile, JSON.stringify(commonJson, null, 2))
 	}
 
-	console.log(chalk.green(`✔ messages.json written for ${browser}.`))
+	ok(`messages.json written for ${browser}.`)
 }
 
 
@@ -303,7 +362,7 @@ function mergeManifest(browser) {
 		JSON.stringify(merged, null, 2)
 	)
 
-	console.log(chalk.green(`✔ manifest.json written for ${browser}.`))
+	ok(`manifest.json written for ${browser}.`)
 }
 
 
@@ -319,18 +378,12 @@ function getPngs(converter, browser) {
 
 
 function renameTestVersion(browser) {
-	try {
-		const changes = replace.sync({
-			files: path.join(pathToBuild(browser), '**', 'messages.json'),
-			from: /"Landmark(s| Navigation via Keyboard or Pop-up)"/g,
-			to: '"Landmarks (test version)"'
-			// Note: Chrome Web Store has a limit of 45 characters for name.
-		})
-		console.log('Suffixed name to indicate test version:',
-			changes.join(', '))
-	} catch (error) {
-		error('Error occurred:', error)
-	}
+	logStep('Changing test version name in messages.json...')
+	doReplace(
+		path.join(pathToBuild(browser), '**', 'messages.json'),
+		/"Landmark(s| Navigation via Keyboard or Pop-up)"/g,
+		'"Landmarks (test version)"',
+		'Suffixed name to indicate test version in:')
 }
 
 
@@ -353,7 +406,7 @@ async function makeZip(browser) {
 	archive.pipe(output)
 	archive.directory(pathToBuild(browser), '')
 	await archive.finalize().then(() => {
-		console.log(chalk.green('✔ ' + archive.pointer() + ' total bytes for ' + outputFileName))
+		ok(archive.pointer() + ' total bytes for ' + outputFileName)
 	})
 }
 
@@ -379,16 +432,6 @@ async function lintFirefox() {
 }
 
 
-/* TODO reinstate or remove
-function copyESLintRC() {
-	logStep('Copying src ESLint config to build directory...')
-	const basename = '.eslintrc.json'
-	fse.copySync(
-		path.join('src', basename),
-		path.join('build', basename))
-} */
-
-
 async function main() {
 	console.log(chalk.bold(`Builing ${extName} ${extVersion}...`))
 	const browsers = checkBuildMode()
@@ -410,9 +453,6 @@ async function main() {
 		await makeZip(browser)
 		await lint(browser)
 	}
-
-	// TODO reinstate or remove
-	// copyESLintRC()
 }
 
 

src/assemble/commonMessages.json

@@ -45,6 +45,10 @@
     "message": "Error"
   },
 
+  "errorNoConnection": {
+    "message": "Landmarks cannot run on this page."
+  },
+
   "errorGettingLandmarksFromContentScript": {
     "message": "Sorry, there was an error when trying to find landmarks"
   },

src/assemble/interfaceMessages.json

@@ -1,13 +1,43 @@
 {
   "prefsInterface": {
-    "message": "Show landmarks in"
+    "message": "Show landmarks in:"
   },
 
   "prefsPopup": {
-    "message": "Toolbar popup"
+    "message": "Toolbar pop-up and sidebar"
+  },
+
+  "prefsPopupExplanation": {
+    "message": "Activating the toolbar button or shortcut key will display landmarks in a pop-up. If you have the sidebar open, they will be shown there too."
   },
 
   "prefsSidebar": {
-    "message": "Sidebar"
+    "message": "Sidebar only"
+  },
+
+  "prefsSidebarExplanation": {
+    "message": "Landmarks will be shown in the sidebar, which can be opened via the toolbar button or shortcut key."
+  },
+
+  "prefsResetMessages": {
+    "message": "Reinstate dismissed messages"
+  },
+
+  "prefsResetMessagesDone": {
+    "message": "Help messages have been reinstated."
+  },
+
+
+
+  "hintSidebarIsNotPrimary": {
+    "message": "Landmarks is set up to use a pop-up. The sidebar will still work, but using the shortcut key will make the pop-up appear, not the sidebar."
+  },
+
+  "hintSidebarIsNotPrimaryOptions": {
+    "message": "Change options"
+  },
+
+  "hintDismiss": {
+    "message": "Dismiss note"
   }
 }

src/assemble/manifest.firefox.json

@@ -15,7 +15,7 @@
 
   "sidebar_action": {
     "default_title": "Landmarks",
-    "default_panel": "popup.html",
+    "default_panel": "panel.html",
     "default_icon": {
       "32": "landmarks-32.png",
       "64": "landmarks-64.png"

src/assemble/manifest.opera.json

@@ -13,7 +13,7 @@
 
   "sidebar_action": {
     "default_title": "Landmarks",
-    "default_panel": "popup.html",
+    "default_panel": "panel.html",
     "default_icon": {
       "19": "landmarks-19.png",
       "38": "landmarks-38.png"

src/code/.eslintrc.json

@@ -1,4 +1,13 @@
 {
+  "env": {
+    "browser": true,
+    "node": false
+  },
+  "globals": {
+    "BROWSER": false,
+    "INTERFACE": false,
+    "browser": false
+  },
   "parserOptions": {
     "sourceType": "module"
   }