Cleanups

Vimarily Extension/ConfigurationModel.swift

@@ -8,10 +8,10 @@ import SafariServices
 class ConfigurationModel: ConfigurationModelProtocol {
 	func getUserSettings() throws -> [String: Any] {
 		var result: [String: Any] = [:]
-		var actionBindings: [String: [String]] = [:]
+		var actionBindings: [String: String] = [:]
 
 		UserDefaults.BindingKeys.allCases.forEach { actionKey in
-			actionBindings[actionKey.rawValue] = SafariExtensionHandler.DEFAULTS_INSTANCE.stringArray(forKey: actionKey.rawValue)
+			actionBindings[actionKey.rawValue] = SafariExtensionHandler.DEFAULTS_INSTANCE.string(forKey: actionKey.rawValue)
 		}
 		result["bindings"] = actionBindings
 

Vimarily Extension/SafariExtensionHandler.swift

@@ -31,10 +31,10 @@ class SafariExtensionHandler: SFSafariExtensionHandler {
 			case .exampleInputAction:
 				NSLog("Example Input Action")
 			case .none:
-				NSLog("Input not supported " + messageName)
+				NSLog("Input not supported \(messageName)")
 			}
 		} catch {
-			NSLog("Message Received " + error.localizedDescription)
+			NSLog("Message Received \(error.localizedDescription)")
 		}
 	}
 

Vimarily Extension/js/lib/svim-scripts.js

@@ -8,11 +8,7 @@ let animationFrame = null;
 
 function customScrollBy(x, y) {
 	// If smooth scroll is off then use regular scroll
-	if (
-		settings == undefined ||
-		settings.smoothScroll === undefined ||
-		!settings.smoothScroll
-	) {
+	if (settings == undefined || !settings.smoothScroll) {
 		window.scrollBy(x, y);
 		return;
 	}

Vimarily Extension/js/lib/vimium-scripts.js

@@ -26,11 +26,6 @@ HUD = {
 		HUD.displayElement().style.display = '';
 	},
 
-	onUpdateLinkClicked: function (event) {
-		HUD.hideUpgradeNotification();
-		chrome.extension.sendRequest({ handler: 'upgradeNotificationClosed' });
-	},
-
 	hideUpgradeNotification: function (clickEvent) {
 		Tween.fade(HUD.upgradeNotificationElement(), 0, 150, function () {
 			HUD.upgradeNotificationElement().style.display = 'none';

Vimarily Extension/js/link-hints.js

@@ -17,7 +17,7 @@ let shouldOpenLinkHintWithQueue = false;
 // Whether link hint's "open in current/new tab" setting is currently toggled
 let openLinkModeToggle = false;
 // Whether we have added to the page the CSS needed to display link hints.
-const linkHintsCssAdded = false;
+let linkHintsCssAdded = false;
 
 // We need this as a top-level function because our command system doesn't yet support arguments.
 function activateLinkHintsModeToOpenInNewTab() {
@@ -30,7 +30,7 @@ function activateLinkHintsModeWithQueue() {
 
 function activateLinkHintsMode(openInNewTab, withQueue) {
 	if (!linkHintsCssAdded) addCssToPage(linkHintCss); // linkHintCss is declared by vimiumFrontend.js
-	linkHintCssAdded = true;
+	linkHintsCssAdded = true;
 	linkHintsModeActivated = true;
 	setOpenLinkMode(openInNewTab, withQueue);
 	buildLinkHints();
@@ -47,30 +47,26 @@ function setOpenLinkMode(openInNewTab, withQueue) {
  * Builds and displays link hints for every visible clickable item on the page.
  */
 function buildLinkHints() {
-	let i;
 	const visibleElements = getVisibleClickableElements();
 
 	// Initialize the number used to generate the character hints to be as many digits as we need to
 	// highlight all the links on the page; we don't want some link hints to have more chars than others.
 	const digitsNeeded = Math.ceil(
 		logXOfBase(visibleElements.length, settings.linkHintCharacters.length)
 	);
-	let linkHintNumber = 0;
-	for (i = 0; i < visibleElements.length; i++) {
-		hintMarkers.push(
-			createMarkerFor(visibleElements[i], linkHintNumber, digitsNeeded)
-		);
-		linkHintNumber++;
-	}
+	visibleElements.forEach((visibleElement, index) => {
+		hintMarkers.push(createMarkerFor(visibleElement, index, digitsNeeded));
+	});
 	// Note(philc): Append these markers as top level children instead of as child nodes to the link itself,
 	// because some clickable elements cannot contain children, e.g. submit buttons. This has the caveat
 	// that if you scroll the page and the link has position=fixed, the marker will not stay fixed.
 	// Also note that adding these nodes to document.body all at once is significantly faster than one-by-one.
 	hintMarkerContainingDiv = document.createElement('div');
 	hintMarkerContainingDiv.id = 'vimiumHintMarkerContainer';
 	hintMarkerContainingDiv.className = 'vimiumReset';
-	for (i = 0; i < hintMarkers.length; i++)
-		hintMarkerContainingDiv.appendChild(hintMarkers[i]);
+	hintMarkers.forEach((hintMarker) => {
+		hintMarkerContainingDiv.appendChild(hintMarker);
+	});
 	document.body.appendChild(hintMarkerContainingDiv);
 }
 
@@ -88,27 +84,14 @@ function getVisibleClickableElements() {
 	const elements = getClickableElements();
 
 	// Get those that are visible too.
-	const visibleElements = [];
-
-	for (let i = 0; i < elements.length; i++) {
-		const element = elements[i];
-
-		const selectedRect = getFirstVisibleRect(element);
-		if (selectedRect) {
-			visibleElements.push(selectedRect);
-		}
-	}
+	const visibleElements = elements.map(getFirstVisibleRect).filter((e) => !!e);
 
 	return visibleElements;
 }
 
 function getClickableElements() {
 	const elements = document.getElementsByTagName('*');
-	const clickableElements = [];
-	for (let i = 0; i < elements.length; i++) {
-		const element = elements[i];
-		if (isClickable(element)) clickableElements.push(element);
-	}
+	const clickableElements = elements.filter(isClickable);
 	return clickableElements;
 }
 
@@ -340,18 +323,18 @@ function isSelectable(element) {
  */
 function highlightLinkMatches(searchString) {
 	const linksMatched = [];
-	for (let i = 0; i < hintMarkers.length; i++) {
-		const linkMarker = hintMarkers[i];
+	hintMarkers.forEach((linkMarker) => {
 		if (linkMarker.getAttribute('hintString').indexOf(searchString) === 0) {
 			if (linkMarker.style.display === 'none') linkMarker.style.display = '';
-			for (let j = 0; j < linkMarker.childNodes.length; j++)
-				linkMarker.childNodes[j].className =
-					j >= searchString.length ? '' : 'matchingCharacter';
+			linkMarker.childNodes.forEach((node, index) => {
+				node.className =
+					index >= searchString.length ? '' : 'matchingCharacter';
+			});
 			linksMatched.push(linkMarker.clickableItem);
 		} else {
 			linkMarker.style.display = 'none';
 		}
-	}
+	});
 	return linksMatched;
 }
 
@@ -415,10 +398,9 @@ function createMarkerFor(link, linkHintNumber, linkHintDigits) {
 	marker.className = 'internalVimiumHintMarker vimiumReset';
 	const innerHTML = [];
 	// Make each hint character a span, so that we can highlight the typed characters as you type them.
-	for (let i = 0; i < hintString.length; i++)
-		innerHTML.push(
-			'<span class="vimiumReset">' + hintString[i].toUpperCase() + '</span>'
-		);
+	hintString.forEach((str) =>
+		innerHTML.push('<span class="vimiumReset">' + str.toUpperCase() + '</span>')
+	);
 	marker.innerHTML = innerHTML.join('');
 	marker.setAttribute('hintString', hintString);
 

Vimarily.xcodeproj/project.pbxproj

@@ -14,7 +14,6 @@
 		3306C4420B6EEF86538E3C51 /* UserDefaults.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306C8B1951F6AA5DDB09D55 /* UserDefaults.swift */; };
 		3306C6815F6B63463ED59505 /* KeyBindingsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306C746646546C49F6BD7BF /* KeyBindingsViewModel.swift */; };
 		3306C6852F586F2B32687159 /* SettingsViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306CBEA276E256F4A1922B9 /* SettingsViewModel.swift */; };
-		3306C802F2BFE301BDC3A495 /* ErrorText.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306CBA39B7B2560138E3487 /* ErrorText.swift */; };
 		3306C93CC71A8B923B81BC0D /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306C652F86CFAB0D3B39D5D /* SettingsView.swift */; };
 		3306C9CB36E11EF48B4D7B49 /* vimarily.spec.js in Sources */ = {isa = PBXBuildFile; fileRef = 3306C2C9ECE7975AF09154BA /* vimarily.spec.js */; };
 		3306CAA21AF06A71019E12CE /* GeneralView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3306C0A2D14E792D9C8AAE35 /* GeneralView.swift */; };
@@ -96,7 +95,6 @@
 		3306C652F86CFAB0D3B39D5D /* SettingsView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsView.swift; sourceTree = "<group>"; };
 		3306C746646546C49F6BD7BF /* KeyBindingsViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = KeyBindingsViewModel.swift; sourceTree = "<group>"; };
 		3306C8B1951F6AA5DDB09D55 /* UserDefaults.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UserDefaults.swift; sourceTree = "<group>"; };
-		3306CBA39B7B2560138E3487 /* ErrorText.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ErrorText.swift; sourceTree = "<group>"; };
 		3306CBEA276E256F4A1922B9 /* SettingsViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SettingsViewModel.swift; sourceTree = "<group>"; };
 		3306CD34A53468F058B434BC /* VimarilyExtensionDebug.entitlements */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.entitlements; path = VimarilyExtensionDebug.entitlements; sourceTree = "<group>"; };
 		3306CDEFFFEA5FD65D6A262C /* GeneralViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = GeneralViewModel.swift; sourceTree = "<group>"; };
@@ -168,7 +166,6 @@
 				3306CAB3E83870AC70F4B64F /* KeyBindingsView */,
 				3306C3F06279AA8F67E0FC73 /* SettingsView */,
 				3306C19BF2DAD7AFF62D204F /* ReloadReminder.swift */,
-				3306CBA39B7B2560138E3487 /* ErrorText.swift */,
 			);
 			path = Views;
 			sourceTree = "<group>";
@@ -425,7 +422,6 @@
 				3306C93CC71A8B923B81BC0D /* SettingsView.swift in Sources */,
 				3306C6852F586F2B32687159 /* SettingsViewModel.swift in Sources */,
 				3306C160AAEF10C3BFA4E001 /* ReloadReminder.swift in Sources */,
-				3306C802F2BFE301BDC3A495 /* ErrorText.swift in Sources */,
 				3306C0FACC0EA21459A89F74 /* UserDefaults.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;

Vimarily/Models/UserDefaults.swift

@@ -2,7 +2,7 @@ import Foundation
 
 
 extension UserDefaults {
-	static let sharedAppGroup = "group." + (Bundle.main.object(forInfoDictionaryKey: "ParentAppBundleIdentifier") as? String ?? "")
+	static let sharedAppGroup = "group.\(Bundle.main.object(forInfoDictionaryKey: "ParentAppBundleIdentifier") ?? "")"
 	static let INSTANCE = UserDefaults(suiteName: UserDefaults.sharedAppGroup)!
 
 	enum GeneralKeys: String, CaseIterable {
@@ -97,25 +97,25 @@ extension UserDefaults {
 	}
 
 	func resetKeyBindings() {
-		set(["f"], forKey: .bindingKey(.hintToggleBinding))
-		set(["shift+f"], forKey: .bindingKey(.newTabHintToggleBinding))
-		set(["k"], forKey: .bindingKey(.scrollUpBinding))
-		set(["j"], forKey: .bindingKey(.scrollDownBinding))
-		set(["h"], forKey: .bindingKey(.scrollLeftBinding))
-		set(["l"], forKey: .bindingKey(.scrollRightBinding))
-		set(["u"], forKey: .bindingKey(.scrollUpHalfPageBinding))
-		set(["d"], forKey: .bindingKey(.scrollDownHalfPageBinding))
-		set(["g g"], forKey: .bindingKey(.goToPageTopBinding))
-		set(["shift+g"], forKey: .bindingKey(.goToPageBottomBinding))
-		set(["g i"], forKey: .bindingKey(.goToFirstInputBinding))
-		set(["shift+j"], forKey: .bindingKey(.goBackBinding))
-		set(["shift+k"], forKey: .bindingKey(.goForwardBinding))
-		set(["r"], forKey: .bindingKey(.reloadBinding))
-		set(["w"], forKey: .bindingKey(.tabBackBinding))
-		set(["q"], forKey: .bindingKey(.tabForwardBinding))
-		set(["x"], forKey: .bindingKey(.closeTabBinding))
-		set(["t"], forKey: .bindingKey(.openTabBinding))
-		set(["y t"], forKey: .bindingKey(.duplicateTabBinding))
-		set(["y y"], forKey: .bindingKey(.copyUrlBinding))
+		set("f", forKey: .bindingKey(.hintToggleBinding))
+		set("shift+f", forKey: .bindingKey(.newTabHintToggleBinding))
+		set("k", forKey: .bindingKey(.scrollUpBinding))
+		set("j", forKey: .bindingKey(.scrollDownBinding))
+		set("h", forKey: .bindingKey(.scrollLeftBinding))
+		set("l", forKey: .bindingKey(.scrollRightBinding))
+		set("u", forKey: .bindingKey(.scrollUpHalfPageBinding))
+		set("d", forKey: .bindingKey(.scrollDownHalfPageBinding))
+		set("g g", forKey: .bindingKey(.goToPageTopBinding))
+		set("shift+g", forKey: .bindingKey(.goToPageBottomBinding))
+		set("g i", forKey: .bindingKey(.goToFirstInputBinding))
+		set("shift+j", forKey: .bindingKey(.goBackBinding))
+		set("shift+k", forKey: .bindingKey(.goForwardBinding))
+		set("r", forKey: .bindingKey(.reloadBinding))
+		set("w", forKey: .bindingKey(.tabBackBinding))
+		set("q", forKey: .bindingKey(.tabForwardBinding))
+		set("x", forKey: .bindingKey(.closeTabBinding))
+		set("t", forKey: .bindingKey(.openTabBinding))
+		set("y t", forKey: .bindingKey(.duplicateTabBinding))
+		set("y y", forKey: .bindingKey(.copyUrlBinding))
 	}
 }

Vimarily/Views/GeneralView/GeneralView.swift

@@ -42,14 +42,14 @@ struct GeneralView: View {
 				Image("Logo").resizable().scaledToFit().frame(maxHeight: 256).aspectRatio(contentMode: .fit).padding()
 				VStack(alignment: .center, spacing: 16) {
 					Text("Vimarily").font(.title)
-					Text("Status: " + viewModel.extensionStatus).font(.title2)
+					Text("Status: \(viewModel.extensionStatus)").font(.title2)
 					Button(action: viewModel.fetchExtensionStatus) {
 						Text("Refresh Extension Status")
 					}
 					Button(action: viewModel.openSafariExtensionPreferencesClick) {
 						Text("Open Safari Extension Preferences")
 					}
-					Text("Version: " + (appVersion ?? "Unknown")).font(.footnote)
+					Text("Version: \(appVersion ?? "Unknown")").font(.footnote)
 				}
 					.padding()
 			}

Vimarily/Views/GeneralView/GeneralViewModel.swift

@@ -35,7 +35,7 @@ class GeneralViewModel: ObservableObject {
 		SFSafariApplication.showPreferencesForExtension(
 			withIdentifier: (Bundle.main.bundleIdentifier ?? "") + Constants.extensionIdentifierSuffix) { error in
 			if error != nil {
-				NSLog("Error" + (error?.localizedDescription ?? "Unknown"))
+				NSLog("Error \(error?.localizedDescription ?? "Unknown")")
 			}
 		}
 	}

Vimarily/Views/KeyBindingsView/KeyBindingsView.swift

@@ -14,10 +14,9 @@ struct KeyBindingsView: View {
 				VStack(alignment: .center) {
 					ForEach(UserDefaults.BindingKeys.allCases, id: \.self) { key in
 						TextField(text: Binding(
-							// TODO temp get and set the first index
-							get: { viewModel.actionBindings[key]?[0] ?? "" },
+							get: { viewModel.actionBindings[key] ?? "" },
 							set: {
-								viewModel.actionBindings[key]?[0] = $0
+								viewModel.actionBindings[key] = $0
 							}
 						), label: { Text(key.rawValue) })
 					}

Vimarily/Views/KeyBindingsView/KeyBindingsViewModel.swift

@@ -1,7 +1,7 @@
 import Foundation
 
 class KeyBindingsViewModel: ObservableObject {
-	@Published var actionBindings: [UserDefaults.BindingKeys : [String]]!
+	@Published var actionBindings: [UserDefaults.BindingKeys : String]!
 
 	init() {
 		populate()
@@ -10,7 +10,7 @@ class KeyBindingsViewModel: ObservableObject {
 	func populate() {
 		actionBindings = [:]
 		UserDefaults.BindingKeys.allCases.forEach { actionKey in
-			actionBindings[actionKey] = UserDefaults.INSTANCE.stringArray(forKey: actionKey.rawValue)
+			actionBindings[actionKey] = UserDefaults.INSTANCE.string(forKey: actionKey.rawValue)
 		}
 	}
 

Vimarily/Views/SettingsView/SettingsView.swift

@@ -2,10 +2,16 @@ import Foundation
 import SwiftUI
 import Combine
 
+let numberFormatter: NumberFormatter = {
+    var nf = NumberFormatter()
+    nf.numberStyle = .decimal
+    nf.allowsFloats = false
+    nf.minimum = 0
+    return nf
+}()
+
 struct SettingsView: View {
 	@ObservedObject private var viewModel: SettingsViewModel
-	@State var isScrollSizeValid = true
-	@State var isScrollDurationValid = true
 
 	init(viewModel: SettingsViewModel) {
 		self.viewModel = viewModel
@@ -16,31 +22,10 @@ struct SettingsView: View {
 			ScrollView {
 				VStack(alignment: .leading, spacing: 16) {
 					Section(header: Text("Scroll").font(.title)) {
-						TextField("Scroll Size", text: Binding(
-							get: { String(viewModel.scrollSize) },
-							set: {
-								guard let parsedValue = Int($0) else {
-									self.isScrollSizeValid = false
-									return
-								}
-								self.isScrollSizeValid = true
-								viewModel.scrollSize = parsedValue
-							}
-						))
-						ErrorText(text: "Invalid Scroll Size - Must be a Number", condition: !isScrollSizeValid)
+                        TextField("Scroll Size", value: $viewModel.scrollSize, formatter: numberFormatter)
+
+                        TextField("Scroll Duration", value: $viewModel.scrollDuration, formatter: numberFormatter)
 
-						TextField("Scroll Duration", text: Binding(
-							get: { String(viewModel.scrollDuration) },
-							set: {
-								guard let parsedValue = Int($0) else {
-									self.isScrollDurationValid = false
-									return
-								}
-								self.isScrollDurationValid = true
-								viewModel.scrollDuration = parsedValue
-							}
-						))
-						ErrorText(text: "Invalid Scroll Duration - Must be a Number", condition: !isScrollDurationValid)
 
 						Toggle(isOn: $viewModel.smoothScroll, label: { Text("Smooth Scroll") })
 					}

Vimarily/Views/SettingsView/SettingsViewModel.swift

@@ -29,6 +29,8 @@ class SettingsViewModel: ObservableObject {
 	}
 
 	public func save() {
+        scrollSize = scrollSize ?? 0
+        scrollDuration = scrollDuration ?? 0
 		UserDefaults.INSTANCE.set(excludedUrls, forKey: .generalKey(.excludedUrls))
 		UserDefaults.INSTANCE.set(linkHintCharacters, forKey: .generalKey(.linkHintCharacters))
 		UserDefaults.INSTANCE.set(detectByCursorStyle, forKey: .generalKey(.detectByCursorStyle))