fix(UI): Add help and preferences links to GUIs (#253)

This makes Help and Preferences more discoverable. It involved renaming some messages for clarity, factoring out translation and also improving the styles somewhat, including having slight differences on the sidebar styles (to make it fill height). The links don't show up in the DevTools panel. When the stand-alone options page is done, there should be a big refactoring and harmonisation of styles across options, GUIs and help. Fixes #250.
matatk · Jan 18, 2019 · da628f9 · da628f9
1 parent 2826cf3
commit da628f9
Show file tree

Hide file tree

Showing 11 changed files with 188 additions and 97 deletions.
diff --git a/scripts/build.js b/scripts/build.js
@@ -244,49 +244,54 @@ async function flattenCode(browser) {
 }
 
 
+function removeUIstuff(htmlFile) {
+	doReplace(
+		htmlFile,
+		/<!-- ui -->[\s\S]*?<!-- \/ui -->\s*/g,
+		'',
+		'Removed UI stuff')
+}
+
+
 function copyStaticFiles(browser) {
 	logStep('Copying static files')
 	fse.copySync(srcStaticDir, pathToBuild(browser))
 	fs.unlinkSync(path.join(pathToBuild(browser), '.eslintrc.json'))
 
 	if (browser === 'chrome' || browser === 'edge') {
-		doReplace(
-			path.join(pathToBuild(browser), '*.html'),
-			/<!-- ui -->[\s\S]*?<!-- \/ui -->\s*/g,
-			'',
-			'Removed UI options')
-		doReplace(
-			path.join(pathToBuild(browser), '*.css'),
-			/\/\* ui \*\/[\s\S]*?\/\* \/ui \*\/\s*/g,
-			'',
-			'Removed UI styles')
+		removeUIstuff(path.join(pathToBuild(browser), 'options.html'))
+		fs.unlinkSync(path.join(pathToBuild(browser), 'sidebar.css'))
 	}
 }
 
 
 function copyGuiFiles(browser) {
 	logStep('Copying root GUI HTML to create the popup and other bits')
 
-	function copyOneGuiFile(destination) {
-		fs.copyFileSync(
-			path.join(srcAssembleDir, 'gui.html'),
-			path.join(pathToBuild(browser), `${destination}.html`))
+	function copyOneGuiFile(destination, doRemoveUIstuff) {
+		const destHtml = path.join(pathToBuild(browser), `${destination}.html`)
+
+		fs.copyFileSync(path.join(srcAssembleDir, 'gui.html'), destHtml)
 
 		doReplace(
-			path.join(pathToBuild(browser), `${destination}.html`),
+			destHtml,
 			'GUIJS',
 			`${destination}.js`,
 			`Included ${destination} code`)
+
+		if (doRemoveUIstuff) {
+			removeUIstuff(destHtml)
+		}
 	}
 
-	copyOneGuiFile('popup')
+	copyOneGuiFile('popup', true)
 
 	if (browser === 'firefox' || browser === 'opera') {
-		copyOneGuiFile('sidebarPanel')
+		copyOneGuiFile('sidebarPanel', false)
 	}
 
 	if (browser === 'firefox' || browser === 'chrome' || browser === 'opera') {
-		copyOneGuiFile('devtoolsPanel')
+		copyOneGuiFile('devtoolsPanel', true)
 	}
 }
 

diff --git a/src/assemble/gui.html b/src/assemble/gui.html
@@ -4,14 +4,22 @@
 		<meta charset="UTF-8">
 		<title></title>
 		<link rel="stylesheet" href="gui.css">
+		<!-- ui -->
+		<link rel="stylesheet" href="sidebar.css">
+		<!-- /ui -->
 	</head>
 	<body>
-		<h1 id="heading"></h1>
-		<div id="landmarks"></div>
-		<label id="show-all-label">
-			<input type="checkbox" id="show-all">
-		</label>
-
+		<div id="content">
+			<h1 data-message="popupHeading"></h1>
+			<div id="landmarks"></div>
+			<label id="show-all-label" data-message="popupShowAllLandmarks">
+				<input type="checkbox" id="show-all">
+			</label>
+		</div>
+		<div id="links">
+			<a tabindex="0" id="help" data-message="popupHelpButton"></a>
+			<a tabindex="0" id="settings" data-message="popupPreferencesButton"></a>
+		</div>
 		<script src="GUIJS"></script>
 	</body>
 </html>
diff --git a/src/assemble/messages.common.json b/src/assemble/messages.common.json
@@ -53,18 +53,19 @@
     "message": "No main landmark found."
   },
 
+  "popupHelpButton": {
+    "message": "Help"
+  },
 
-
-
-  "inspectButtonName": {
-    "message": "Inspect"
+  "popupPreferencesButton": {
+    "message": "Preferences"
   },
 
 
 
 
   "prefsTitle": {
-    "message": "Landmarks Options"
+    "message": "Landmarks preferences"
   },
 
   "prefsBorderType": {
@@ -217,6 +218,10 @@
 
 
 
+  "inspectButtonName": {
+    "message": "Inspect"
+  },
+
   "devToolsConnectionError": {
     "message": "This DevTools frame has become disconnected, probably due to reloading the extension. You'll need to reload this frame for it to recieve landmark updates again."
   }

diff --git a/src/code/_background.js b/src/code/_background.js
@@ -100,6 +100,7 @@ function contentListener(message, sendingPort) {
 }
 
 // TODO DRY with devToolsListener
+// TODO DRY with splashListener
 function popupAndSidebarListener(message) {  // also gets: sendingPort
 	switch (message.name) {
 		case 'get-landmarks':
@@ -111,12 +112,21 @@ function popupAndSidebarListener(message) {  // also gets: sendingPort
 		case 'toggle-all-landmarks':
 			sendToActiveContentScriptIfExists(message)
 			break
+		case 'open-help':
+			browser.tabs.update({
+				url: browser.runtime.getURL('help.html')
+			})
+			break
+		case 'open-settings':
+			browser.runtime.openOptionsPage()
+			break
 		default:
 			throw Error(`Unknown message ${JSON.stringify(message)} from popup or sidebar`)
 	}
 }
 
 // TODO DRY with popupAndSideBarListener
+// TODO DRY with splashListener
 function devtoolsListenerMaker(connectingPort) {
 	// DevTools connections come from the DevTools panel, but the panel is
 	// inspecting a particular web page, which has a different tab ID.
@@ -139,36 +149,46 @@ function devtoolsListenerMaker(connectingPort) {
 			case 'toggle-all-landmarks':
 				sendToActiveContentScriptIfExists(message)
 				break
+			case 'open-help':
+				browser.tabs.update({
+					url: browser.runtime.getURL('help.html')
+				})
+				break
+			case 'open-settings':
+				browser.runtime.openOptionsPage()
+				break
 			default:
 				throw Error(`Unknown message from DevTools: ${JSON.stringify(message)}`)
 		}
 	}
 }
 
+// TODO DRY with popupAndSideBarListener
+// TODO DRY with devToolsListener
 function splashListener(message, sendingPort) {
 	switch (message.name) {
 		case 'get-commands':
 			browser.commands.getAll(function(commands) {
 				sendingPort.postMessage({
-					name: 'splash-populate-commands',
+					name: 'populate-commands',
 					commands: commands
 				})
 			})
 			break
-		case 'splash-open-configure-shortcuts':
+		case 'open-configure-shortcuts':
 			browser.tabs.update({
 				// This should only appear on Chrome/Opera
 				url: BROWSER === 'chrome'
 					? 'chrome://extensions/configureCommands'
 					: 'opera://settings/keyboardShortcuts'
 			})
 			break
-		case 'splash-open-help':
+		case 'open-help':
 			browser.tabs.update({
 				url: browser.runtime.getURL('help.html')
 			})
 			break
-		case 'splash-open-settings':
+		case 'open-settings':
 			browser.runtime.openOptionsPage()
 			break
 		default:

diff --git a/src/code/_gui.js b/src/code/_gui.js
@@ -1,4 +1,5 @@
 import './compatibility'
+import translate from './translate'
 import landmarkName from './landmarkName'
 import { defaultInterfaceSettings, dismissalStates } from './defaults'
 import disconnectingPortErrorCheck from './disconnectingPortErrorCheck'
@@ -20,20 +21,21 @@ let port = null
 // an error, let the user know.
 function handleLandmarksMessage(data) {
 	const display = document.getElementById('landmarks')
+	const showAllContainer = document.getElementById('show-all-label')
 	removeChildNodes(display)
 
 	// Content script would normally send back an array of landmarks
 	if (Array.isArray(data)) {
 		if (data.length === 0) {
 			addText(display, browser.i18n.getMessage('noLandmarksFound'))
-			document.getElementById('show-all-label').setAttribute('hidden', '')
+			showAllContainer.style.display = 'none'
 		} else {
 			makeLandmarksTree(data, display)
-			document.getElementById('show-all-label').removeAttribute('hidden')
+			showAllContainer.style.display = null
 		}
 	} else {
 		addText(display, browser.i18n.getMessage('errorNoConnection'))
-		document.getElementById('show-all-label').setAttribute('hidden', '')
+		showAllContainer.style.display = 'none'
 	}
 }
 
@@ -236,7 +238,8 @@ if (INTERFACE === 'sidebar') {
 				note.appendChild(para)
 				note.appendChild(buttons)
 
-				document.body.insertBefore(note, document.body.firstChild)
+				const content = document.getElementById('content')
+				content.insertBefore(note, content.firstChild)
 			}
 		})
 	}
@@ -285,25 +288,39 @@ if (INTERFACE === 'sidebar') {
 // Management
 //
 
+function makeEventHandlers(linkName) {
+	const link = document.getElementById(linkName)
+	const core = () => {
+		port.postMessage({ name: `open-${linkName}` })
+		if (INTERFACE === 'popup') window.close()
+	}
+
+	link.addEventListener('click', core)
+	link.addEventListener('keydown', function(event) {
+		if (event.key === 'Enter') core()
+	})
+}
+
 // When the pop-up (or sidebar) opens, translate the heading and grab and
 // process the list of page landmarks
 function main() {
-	document.getElementById('heading').innerText =
-		browser.i18n.getMessage('popupHeading')
-
-	document.getElementById('show-all-label').appendChild(document
-		.createTextNode(browser.i18n.getMessage('popupShowAllLandmarks')))
-
 	if (INTERFACE === 'devtools') {
+		document.getElementById('links').remove()
+
 		port = browser.runtime.connect({ name: INTERFACE })
 		port.postMessage({
 			name: 'init',
 			tabId: browser.devtools.inspectedWindow.tabId
 		})
 	} else {
+		makeEventHandlers('help')
+		makeEventHandlers('settings')
+
 		port = browser.runtime.connect({ name: INTERFACE })
 	}
 
+	translate()
+
 	port.onDisconnect.addListener(function() {
 		disconnectingPortErrorCheck()
 		// Note: Firefox doesn't use 'devToolsConnectionError' but if it is not

diff --git a/src/code/_help.js b/src/code/_help.js
@@ -21,14 +21,14 @@ const keyboardShortcutsLink = {
 	listen: [{
 		event: 'click',
 		handler: () => port.postMessage({
-			name: 'splash-open-configure-shortcuts'
+			name: 'open-configure-shortcuts'
 		})
 	}, {
 		event: 'keydown',
 		handler: (event) => {
 			if (event.key === 'Enter') {
 				port.postMessage({
-					name: 'splash-open-configure-shortcuts'
+					name: 'open-configure-shortcuts'
 				})
 			}
 		}
@@ -44,15 +44,15 @@ const settingsLink = {
 		event: 'click',
 		handler: () => {
 			port.postMessage({
-				name: 'splash-open-settings'
+				name: 'open-settings'
 			})
 		}
 	}, {
 		event: 'keydown',
 		handler: (event) => {
 			if (event.key === 'Enter') {
 				port.postMessage({
-					name: 'splash-open-configure-shortcuts'
+					name: 'open-configure-shortcuts'
 				})
 			}
 		}
@@ -160,7 +160,7 @@ function firefoxShortcutElements(shortcut) {
 }
 
 function messageHandler(message) {  // also sendingPort
-	if (message.name !== 'splash-populate-commands') return
+	if (message.name !== 'populate-commands') return
 
 	// Chrome allows only four keyboard shortcuts to be specified in the
 	// manifest; Firefox allows many.

diff --git a/src/code/_options.js b/src/code/_options.js
@@ -1,4 +1,5 @@
 import './compatibility'
+import translate from './translate'
 import { defaultSettings, dismissalStates } from './defaults'
 
 const options = [{
@@ -19,18 +20,6 @@ const options = [{
 	property: 'checked'
 }]
 
-// Translation
-// http://tumble.jeremyhubert.com/post/7076881720
-// HT http://stackoverflow.com/questions/25467009/
-function translateStuff() {
-	const objects = document.getElementsByTagName('*')
-	for(const object of objects) {
-		if (object.dataset && object.dataset.message) {
-			object.innerText = browser.i18n.getMessage(object.dataset.message)
-		}
-	}
-}
-
 function restoreOptions() {
 	browser.storage.sync.get(defaultSettings, function(items) {
 		for (const option of options) {
@@ -139,7 +128,7 @@ function main() {
 		})
 	}
 
-	translateStuff()
+	translate()
 	restoreOptions()
 	setUpOptionHandlers()
 }

diff --git a/src/code/translate.js b/src/code/translate.js
@@ -0,0 +1,8 @@
+// http://tumble.jeremyhubert.com/post/7076881720
+// HT http://stackoverflow.com/questions/25467009/
+export default function translate() {
+	for(const element of document.querySelectorAll('[data-message]')) {
+		element.appendChild(document.createTextNode(
+			browser.i18n.getMessage(element.dataset.message)))
+	}
+}