Comment browser code + small (code) style changes

grork · Jul 28, 2022 · f166399 · f166399
1 parent 496428d
commit f166399
Show file tree

Hide file tree

Showing 4 changed files with 252 additions and 70 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,5 +3,10 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## 1.1 (2022-07-28)
+Added a built in browser based on VS Code's Simple Browser
+- Enabled availability checks before opening browser
+- Added checkbox to enable cache bypass in the browser
+
 ## 1.0.0 (2022-04-24)
 Initial release
diff --git a/browser/browserUi.ts b/browser/browserUi.ts
@@ -1,18 +1,52 @@
 const vscode = acquireVsCodeApi();
 
+/**
+ * Messages sent to the host of this iframe
+ */
 enum ToHostMessageType {
+    /**
+     * Open the current URL in the system browser
+     */
     OpenInSystemBrowser = "open-in-system-browser",
+
+    /**
+     * The user intiated a setting change for the automatic browser cache bypass
+     */
     AutomaticBrowserCacheBybassStateChanged = "automatic-browser-cache-bypass-setting-changed"
 }
 
+/**
+ * Messages recieved from the host of this iframe
+ */
 enum FromHostMessageType {
+    /**
+     * The focus lock indicator setting has changed
+     */
     FocusIndicatorLockEnabledStateChanged = "focus-lock-indicator-setting-changed",
+
+    /**
+     * The automatic browser cache bypass setting has changed
+     */
     AutomaticBrowserCacheBybassStateChanged = "automatic-browser-cache-bypass-setting-changed",
+
+    /**
+     * Force an refresh of the focus lock state
+     */
+    RefreshFocusLockState = "refresh-focus-lock-state",
+
+    /**
+     * Open a URL in our browser
+     */
     NavigateToUrl = "navigate-to-url"
 }
 
 const CACHE_BYPASS_PARAMETER_NAME = "ilbCacheBypassSecretParameter";
 
+/**
+ * Settings are (intially) passed in the html body as a metatag; this grabs it
+ * from there, and turns it into a real instance/
+ * @returns The settings object
+ */
 function extractSettingsFromMetaTag(): { url: string; focusLockIndicator: boolean;  automaticBrowserCacheBypass: boolean } {
     const element = document.getElementById("browser-settings");
     if (element) {
@@ -29,35 +63,46 @@ function toggleFocusLockIndicator() {
     document.body.classList.toggle("enable-focus-lock-indicator", settings.focusLockIndicator);
 }
 
-function toggleAutomaticBrowserCacheBypassButton() {
+function updateAutomaticBrowserCacheBypassCheckboxState() {
     bypassCacheCheckbox.checked = settings.automaticBrowserCacheBypass;
 }
 
+/**
+ * Sets the address bar to the currentlly set iframe URL. Intended to be used
+ * when someone has changed the address bar, but we didn't navigate to the URL
+ * they typed in
+ */
 function resetAddressBarToCurrentIFrameValue()
 {
     const iframeUrl = new URL(contentIframe.src);
     iframeUrl.searchParams.delete(CACHE_BYPASS_PARAMETER_NAME);
-    locationBar.value = iframeUrl.toString();
+    addressBar.value = iframeUrl.toString();
 }
 
 const settings = extractSettingsFromMetaTag();
 
+// Locate all the buttons & elements we work with
 const contentIframe = document.querySelector("iframe")!;
-const locationBar = document.querySelector<HTMLInputElement>(".url-input")!;
+const addressBar = document.querySelector<HTMLInputElement>(".url-input")!;
 const forwardButton = document.querySelector<HTMLButtonElement>(".forward-button")!;
 const backButton = document.querySelector<HTMLButtonElement>(".back-button")!;
 const bypassCacheCheckbox = document.querySelector<HTMLInputElement>("#bypassCacheCheckbox")!;
 const reloadButton = document.querySelector<HTMLButtonElement>(".reload-button")!;
 const openExternalButton = document.querySelector<HTMLButtonElement>(".open-external-button")!;
 
+/**
+ * Navigate the iframe to the supplied URL, including automatically appending
+ * cache bypass parameters if needed
+ * @param url URL to navigate to
+ */
 function navigateTo(url: URL): void {
+    // Delete the cache bypass parameter if it's present
     if (url.searchParams.has(CACHE_BYPASS_PARAMETER_NAME)) {
         url.searchParams.delete(CACHE_BYPASS_PARAMETER_NAME);
     }
 
-    const nakedUrl = url.toString();
-    vscode.setState({ url: nakedUrl });
-    locationBar.value = nakedUrl;
+    // Save the state in the host
+    vscode.setState({ url: url.toString() });
 
     // Try to bust the cache for the iframe There does not appear to be any way
     // to reliably do this except modifying the url
@@ -66,8 +111,58 @@ function navigateTo(url: URL): void {
     }
 
     contentIframe.src = url.toString();
+    resetAddressBarToCurrentIFrameValue();
+}
+
+/**
+ * Process a user change of address bar text. This will attempt to check that
+ * the URL is valid, and only navigate if it is. If the URL doesn't include the
+ * scheme, we will attempt to add one by default (http if local host, https
+ * otherwise)
+ */
+function handleAddressBarChange(e: Event) {
+    let rawUrl = (<HTMLInputElement>e.target).value;
+    let parsedUrl: URL | null = null;
+
+    // Try to parse it
+    try {
+        parsedUrl = new URL(rawUrl);
+    } catch {
+        try {
+            // Since it wasn't a successful URL, lets try adding a scheme
+            if (!/^https?:\/\//.test(rawUrl)) {
+                if (rawUrl.startsWith("localhost/") || rawUrl.startsWith("localhost:")) {
+                    // default to http for localhost
+                    rawUrl = "http://" + rawUrl;
+                } else {
+                    rawUrl = "https://" + rawUrl;
+                }
+
+                // Try parsing it again
+                parsedUrl = new URL(rawUrl);
+            }
+        } catch { /* Not parsable */ }
+    }
+
+    if (!parsedUrl || (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:")) {
+        resetAddressBarToCurrentIFrameValue();
+        return;
+    }
+
+    navigateTo(parsedUrl!);
+}
+
+/**
+ * Refresh the display of the focus lock state indicator based on the iframe
+ * focus state
+ */
+function refreshFocusLockState(): void {
+    const iframeFocused = document.activeElement?.tagName === "IFRAME";
+    document.body.classList.toggle("iframe-focused", iframeFocused);
+    console.log(`Focused: ${iframeFocused}`);
 }
 
+// Listen for host-sent messages
 window.addEventListener("message", e => {
     switch (e.data.type) {
         case FromHostMessageType.FocusIndicatorLockEnabledStateChanged:
@@ -81,48 +176,27 @@ window.addEventListener("message", e => {
 
         case FromHostMessageType.AutomaticBrowserCacheBybassStateChanged:
             settings.automaticBrowserCacheBypass = e.data.automaticBrowserCacheBypass;
-            toggleAutomaticBrowserCacheBypassButton();
+            updateAutomaticBrowserCacheBypassCheckboxState();
+            break;
+
+        case FromHostMessageType.RefreshFocusLockState:
+            refreshFocusLockState();
             break;
     }
 });
 
 document.addEventListener("DOMContentLoaded", () => {
     toggleFocusLockIndicator();
 
-    setInterval(() => {
-        const iframeFocused = document.activeElement?.tagName === "IFRAME";
-        document.body.classList.toggle("iframe-focused", iframeFocused);
-    }, 50);
+    // Handle focus events in the window so we can correctly indicate of focus
+    // is captured by the iframe itself
+    window.addEventListener("focus", refreshFocusLockState);
+    window.addEventListener("blur", refreshFocusLockState);
 
-    locationBar.addEventListener("change", e => {
-        let rawUrl = (<HTMLInputElement>e.target).value;
-        let parsedUrl: URL | null = null;
-
-        try {
-            parsedUrl = new URL(rawUrl);
-        } catch {
-            try {
-                if (!/^https?:\/\//.test(rawUrl)) {
-                    if (rawUrl.startsWith("localhost/") || rawUrl.startsWith("localhost:")) {
-                        // default to http
-                        rawUrl = "http://" + rawUrl;
-                    } else {
-                        rawUrl = "https://" + rawUrl;
-                    }
-
-                    parsedUrl = new URL(rawUrl);
-                }
-            } catch { /* Not parsable */ }
-        }
-
-        if (!parsedUrl || (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:")) {
-            resetAddressBarToCurrentIFrameValue();
-            return;
-        }
-
-        navigateTo(parsedUrl!);
-    });
+    // When the user commits a change in the address bar, handle it
+    addressBar.addEventListener("change", handleAddressBarChange);
 
+    // Handle changes to the cache bypass checkbox
     bypassCacheCheckbox?.addEventListener("change", (e) => {
         const isChecked = (<HTMLInputElement>e.target).checked;
         vscode.postMessage({
@@ -143,7 +217,7 @@ document.addEventListener("DOMContentLoaded", () => {
     openExternalButton.addEventListener("click", () => {
         vscode.postMessage({
             type: ToHostMessageType.OpenInSystemBrowser,
-            url: locationBar.value
+            url: addressBar.value
         });
     });
 

diff --git a/src/browserManager.ts b/src/browserManager.ts
@@ -1,12 +1,18 @@
 import * as vscode from "vscode";
 import { ShowOptions, BrowserView, BROWSER_VIEW_TYPE } from "./browserView";
 
+/**
+ * Captured state from a previously opened webview
+ */
 interface WebViewPersistedState {
     url?: string;
 }
 
+/**
+ * Manages the active browser view, including registering for certain IDE-level
+ * events to help with restoration
+ */
 export class BrowserManager {
-
     private _activeView?: BrowserView;
 
     constructor(
@@ -18,7 +24,14 @@ export class BrowserManager {
         this._activeView = undefined;
     }
 
+    /**
+     * Show a specific URL in the browser. Only one will be displayed at a time
+     * @param url URL to display
+     * @param options How the browser should be displayed
+     */
     public show(url: string, options?: ShowOptions): void {
+        // If we already have a view, we should ask it to show the URL, rather
+        // than creating a new browser
         if (this._activeView) {
             this._activeView.show(url, options);
             return;
@@ -28,13 +41,19 @@ export class BrowserManager {
         this.registerWebviewListeners(view);
     }
 
+    /**
+     * Handle IDE-driven restoration of a previously open browser
+     */
     public restore(panel: vscode.WebviewPanel, state: WebViewPersistedState): Thenable<void> {
         const url = state?.url;
+
+        // Give up if we the URL we're being asked to restore is not parsable
         if (!url) {
             panel.dispose();
             return Promise.resolve();
         }
 
+        // Supply the **existing** panel, which we're going to restore into
         const view = BrowserView.create(this.extensionUri, url, undefined, panel);
         this.registerWebviewListeners(view);
 
@@ -51,7 +70,11 @@ export class BrowserManager {
         this._activeView = view;
     }
 
-    public handleExtensionActivation(context: vscode.ExtensionContext) {
+    /**
+     * Listen for IDE-level events
+     * @param context Extension context
+     */
+    public handleExtensionActivation(context: vscode.ExtensionContext): void {
         context.subscriptions.push(
             vscode.window.registerWebviewPanelSerializer(
             BROWSER_VIEW_TYPE, {