From 551ed93f10e1ad6606ec5d82c933b2f6ba2a5fce Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Mon, 23 Sep 2024 16:28:01 +0300 Subject: [PATCH] WIP - navigation experiment - let's git rid of our state objects (but keep pushState and replaceState in the right places). - let's trigger connections explicitly instead of letting navigate do it. --- pkg/shell/base_index.js | 103 ++++++++++------------------------------ pkg/shell/failures.jsx | 13 +---- pkg/shell/hosts.jsx | 25 ++++++---- pkg/shell/nav.jsx | 7 +-- pkg/shell/router.jsx | 10 ++-- pkg/shell/shell.jsx | 29 ++++++++--- pkg/shell/state.jsx | 66 ++++++++++++++----------- pkg/shell/util.jsx | 86 +++++++++++++++++++++++++++++++++ 8 files changed, 199 insertions(+), 140 deletions(-) create mode 100644 pkg/shell/util.jsx diff --git a/pkg/shell/base_index.js b/pkg/shell/base_index.js index 1e6e12e3c7e5..35e23fd44b1e 100644 --- a/pkg/shell/base_index.js +++ b/pkg/shell/base_index.js @@ -20,6 +20,7 @@ import cockpit from "cockpit"; import { Router } from "./router.jsx"; +import { encode_state, decode_state } from "./util.jsx"; const shell_embedded = window.location.pathname.indexOf(".html") !== -1; @@ -57,72 +58,13 @@ function Index() { return false; }; - /* - * Navigation is driven by state objects, which are used with pushState() - * and friends. The state is the canonical navigation location, and not - * the URL. Only when no state has been pushed or we are arriving from - * a link, do we parse the state from the URL. - * - * Each state object has: - * host: a machine host - * component: the stripped component to load - * hash: the hash to pass to the component - * sidebar: set to true to hint that we want a component with a sidebar - * - * If state.sidebar is set, and no component has yet been chosen for the - * given state, then we try to find one that would show a sidebar. - */ - - /* Encode navigate state into a string - * If with_root is true the configured - * url root will be added to the generated - * url. with_root should be used when - * navigating to a new url or updating - * history, but is not needed when simply - * generating a string for a link. - */ - function encode(state, sidebar, with_root) { - const path = []; - if (state.host && (sidebar || state.host !== "localhost")) - path.push("@" + state.host); - if (state.component) - path.push.apply(path, state.component.split("/")); - let string = cockpit.location.encode(path, null, with_root); - if (state.hash && state.hash !== "/") - string += "#" + state.hash; - return string; - } - - /* Decodes navigate state from a string */ - function decode(string) { - const state = { version: "v1", hash: "" }; - const pos = string.indexOf("#"); - if (pos !== -1) { - state.hash = string.substring(pos + 1); - string = string.substring(0, pos); - } - if (string[0] != '/') - string = "/" + string; - const path = cockpit.location.decode(string); - if (path[0] && path[0][0] == "@") { - state.host = path.shift().substring(1); - state.sidebar = true; - } else { - state.host = "localhost"; - } - if (path.length && path[path.length - 1] == "index") - path.pop(); - state.component = path.join("/"); - return state; - } - self.retrieve_state = function() { let state = window.history.state; if (!state || state.version !== "v1") { if (shell_embedded) - state = decode("/" + window.location.hash); + state = decode_state("/" + window.location.hash); else - state = decode(window.location.pathname + window.location.hash); + state = decode_state(window.location.pathname + window.location.hash); } return state; }; @@ -138,12 +80,23 @@ function Index() { return null; } + self.track_hash = function track_hash(address, component, hash) { + if (!last_hash_for_host_fullpath[address]) + last_hash_for_host_fullpath[address] = { }; + last_hash_for_host_fullpath[address][component] = hash; + }; + self.last_component_for_host = { }; /* Jumps to a given navigate state */ self.jump = function (state, replace) { + if (replace) + return; + + console.log("JUMP", JSON.stringify(state)); + if (typeof (state) === "string") - state = decode(state); + state = decode_state(state); const current = self.retrieve_state(); @@ -168,34 +121,23 @@ function Index() { if (frame_change && !state.hash) state.hash = lookup_component_hash(state.host, state.component); - if (!last_hash_for_host_fullpath[state.host]) - last_hash_for_host_fullpath[state.host] = { }; - last_hash_for_host_fullpath[state.host][state.component] = state.hash; + self.track_hash(state.host, state.component, state.hash); - const target = shell_embedded ? window.location : encode(state, null, true); - - if (replace) { - history.replaceState(state, "", target); - return false; - } + const target = shell_embedded ? window.location : encode_state(state, null, true); if (frame_change || state.hash !== current.hash) { + console.log("PUSH", JSON.stringify(state), target); history.pushState(state, "", target); const nav_system = document.getElementById("nav-system"); if (nav_system) nav_system.classList.remove("interact"); - self.navigate(state, true); + self.navigate(); return true; } return false; }; - /* Build an href for use in an */ - self.href = function (state, sidebar) { - return encode(state, sidebar); - }; - self.show_oops = function () { self.has_oops = true; self.dispatchEvent("update"); @@ -220,10 +162,13 @@ function Index() { self.ready = function () { window.addEventListener("popstate", ev => { - self.navigate(ev.state, true); + console.log("POP!"); + self.navigate(); + self.ensure_connection(); }); - self.navigate(null, true); + self.navigate(); + self.ensure_connection(); cockpit.translate(); document.body.removeAttribute("hidden"); }; diff --git a/pkg/shell/failures.jsx b/pkg/shell/failures.jsx index da336cd91400..f7493c417ffb 100644 --- a/pkg/shell/failures.jsx +++ b/pkg/shell/failures.jsx @@ -54,16 +54,7 @@ export const EarlyFailure = ({ ca_cert_url }) => { ); }; -export const EarlyFailureReady = ({ loading, title, paragraph, reconnect, troubleshoot, onTroubleshoot, watchdog_problem, navigate }) => { - const onReconnect = () => { - if (watchdog_problem) { - cockpit.sessionStorage.clear(); - window.location.reload(true); - } else { - navigate(null, true); - } - }; - +export const EarlyFailureReady = ({ loading, title, paragraph, reconnect, troubleshoot, onTroubleshoot, watchdog_problem, onReconnect }) => { return ( @@ -117,7 +108,7 @@ export const MachineTroubleshoot = ({ machine, onClick }) => { reconnect={reconnect} troubleshoot={troubleshooting} onTroubleshoot={onClick} - navigate={onClick} + onReconnect={onClick} paragraph={message} /> ); }; diff --git a/pkg/shell/hosts.jsx b/pkg/shell/hosts.jsx index afe90d9acf85..a27e0a3f0cc3 100644 --- a/pkg/shell/hosts.jsx +++ b/pkg/shell/hosts.jsx @@ -16,7 +16,7 @@ import { Tooltip } from "@patternfly/react-core/dist/esm/components/Tooltip"; import 'polyfills'; import { CockpitNav, CockpitNavItem } from "./nav.jsx"; import { HostModal, try2Connect, codes } from "./hosts_dialog.jsx"; -import { useLoggedInUser } from "hooks"; +import { build_href } from "./util.jsx"; const _ = cockpit.gettext; @@ -93,7 +93,11 @@ export class CockpitHosts extends React.Component { if (!this.state.modal_properties) this.connectHost(machine); }; - this.props.index.navigate(null, true); + // XXX - Only because trigger_connection_flow isn't available + // until we are mounted. If that is fixed, remove this + // here. + this.props.index.navigate(); + this.props.index.ensure_connection(); } componentWillUnmount() { @@ -141,7 +145,7 @@ export class CockpitHosts extends React.Component { const connection_string = await this.showModal({ address: machine.address }); if (connection_string) { const parts = this.props.machines.split_connection_string(connection_string); - const addr = this.props.hostAddr({ host: parts.address }, true); + const addr = build_href({ host: parts.address }); if (machine == this.props.machine && parts.address != machine.address) { this.props.loader.connect(parts.address); this.props.jump(addr); @@ -150,9 +154,14 @@ export class CockpitHosts extends React.Component { } async connectHost(machine) { - if (machine.address == "localhost" || machine.state == "connected" || machine.state == "connecting") + if (machine.state == "connected" || machine.state == "connecting") return machine.connection_string; + if (machine.connection_string == "localhost") { + this.props.loader.connect(machine.address); + return; + } + let connection_string = null; if (machine.problem && codes[machine.problem]) { @@ -196,7 +205,7 @@ export class CockpitHosts extends React.Component { const connection_string = await this.connectHost(machine); if (connection_string) { const parts = this.props.machines.split_connection_string(connection_string); - const addr = this.props.hostAddr({ host: parts.address }, true); + const addr = build_href({ host: parts.address }); this.props.jump(addr); } } @@ -210,7 +219,7 @@ export class CockpitHosts extends React.Component { if (this.props.machine === machine) { // Removing machine underneath ourself - jump to localhost - const addr = this.props.hostAddr({ host: "localhost" }, true); + const addr = build_href({ host: "localhost" }); this.props.jump(addr); } @@ -241,7 +250,6 @@ export class CockpitHosts extends React.Component { // 1. It does not change the arrow when opened/closed // 2. It closes the dropdown even when trying to search... and cannot tell it not to render() { - const hostAddr = this.props.hostAddr; const editing = this.state.editing; const groups = [{ name: _("Hosts"), @@ -250,7 +258,7 @@ export class CockpitHosts extends React.Component { const render = (m, term) => { @@ -253,7 +255,6 @@ export const PageNav = ({ component, compiled, page_status, - build_href, jump }) => { if (!machine || machine.state != "connected") { @@ -333,7 +334,7 @@ export const PageNav = ({ status={status} keyword={item.keyword.keyword} term={term} - to={build_href({ host: machine.address, component: path, hash })} + to={build_href({ host: machine.address, component: path, hash }, false)} jump={jump} /> ); } @@ -354,7 +355,7 @@ export const PageNav = ({ if (compiled.items.apps && groups.length === 3) groups[0].action = { label: _("Edit"), - path: build_href({ host: machine.address, component: compiled.items.apps.path }) + path: build_href({ host: machine.address, component: compiled.items.apps.path }, false) }; return { return null; console.log("SHELL", machine.address, component, fullpath, state.hash); - console.log("FRAMES", JSON.stringify(Object.keys(frames))); + // console.log("FRAMES", JSON.stringify(Object.keys(frames))); const title_parts = []; const item = compiled.items[component]; @@ -147,6 +162,10 @@ const Shell = () => { { + cockpit.sessionStorage.clear(); + window.location.reload(true); + }} paragraph={cockpit.message(problem)} /> ); @@ -177,7 +196,6 @@ const Shell = () => { component={component} compiled={compiled} page_status={page_status} - build_href={state.index.href} jump={state.index.jump} /> @@ -189,7 +207,6 @@ const Shell = () => { index={state.index} loader={state.loader} selector="nav-hosts" - hostAddr={state.index.href} jump={state.index.jump} /> : } diff --git a/pkg/shell/state.jsx b/pkg/shell/state.jsx index 215b7e0099e1..04349572f93f 100644 --- a/pkg/shell/state.jsx +++ b/pkg/shell/state.jsx @@ -20,6 +20,8 @@ import cockpit from "cockpit"; import { machines as machines_factory } from "./machines/machines.js"; +import { decode_window_location, replace_window_location } from "./util.jsx"; + import * as base_index from "./base_index"; /* XXX - This is a hacked up version of the old indexes.jsx, without @@ -265,8 +267,12 @@ export function ShellState(trigger_connection_flow) { cockpit.event_target(self); const index_options = { - navigate: function (state, sidebar) { - return navigate(state, sidebar); + navigate: function () { + return navigate(); + }, + + ensure_connection: function () { + ensure_connection(); }, handle_notifications: function (host, page, data) { @@ -329,10 +335,8 @@ export function ShellState(trigger_connection_flow) { if (!machine.visible || machine.problem) remove_machine_frames(machine); else { - update(); preload_frames(); - if (ready) - navigate(); + update(); } }); }); @@ -341,61 +345,65 @@ export function ShellState(trigger_connection_flow) { on_ready(); /* Handles navigation */ - function navigate(state, reconnect) { + function navigate() { if (self.problem) return; - if (!state) - state = index.retrieve_state(); + const location = decode_window_location(); + + console.log("NAV", JSON.stringify(location), window.location.href); // Force a redirect to localhost when the host switcher is // disabled. That way, people won't accidentally connect to // remote machines via URL bookmarks or similar that point to // them. - if (!self.config.host_switcher_enabled) - state.host = "localhost"; + if (!self.config.host_switcher_enabled) { + location.host = "localhost"; + replace_window_location(location); + } - let machine = machines.lookup(state.host); + let machine = machines.lookup(location.host); /* No such machine */ if (!machine) { machine = { - key: state.host, - address: state.host, - label: state.host, + key: location.host, + address: location.host, + label: location.host, state: "failed", problem: "not-found", }; } else if (!machine.visible) { machine.state = "failed"; machine.problem = "not-found"; - } else if (reconnect) { - /* Asked to reconnect to the machine */ - if (state.host == "localhost") { - loader.connect(state.host); - } else { - trigger_connection_flow(machine); - } } const compiled = compile(machine); - if (machine.manifests && !state.component) - state.component = choose_component(state, compiled); + if (machine.manifests && !location.component) { + location.component = choose_component(location, compiled); + replace_window_location(location); + } self.machine = machine; - self.fullpath = state.component; - self.hash = state.hash; + self.fullpath = location.component; + self.hash = location.hash; self.compiled = compiled; self.component = find_component(self.fullpath, compiled); - update_frame(machine, state, compiled.items[self.component]?.label); - - /* Just replace the state, and URL */ - index.jump(state, true); + update_frame(machine, location, compiled.items[self.component]?.label); update(); } + function ensure_connection() { + const machine = self.machine; + if (machine && machine.state != "connected" && machine.state != "connecting" && machine.state != "failed") { + console.log("CONNECTING", machine.address); + trigger_connection_flow(machine); + } else + console.log("NOT connecting to", machine.connection_string, machine.state); + } + function update_frame(machine, state, title) { let current_frame = index.current_frame(); diff --git a/pkg/shell/util.jsx b/pkg/shell/util.jsx new file mode 100644 index 000000000000..f34093571bf4 --- /dev/null +++ b/pkg/shell/util.jsx @@ -0,0 +1,86 @@ +/* + * This file is part of Cockpit. + * + * Copyright (C) 2024 Red Hat, Inc. + * + * Cockpit is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; either version 2.1 of the License, or + * (at your option) any later version. + * + * Cockpit is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Cockpit; If not, see . + */ + +import cockpit from "cockpit"; + +/* Encode navigate state into a string If with_root is true the + * configured url root will be added to the generated url. with_root + * should be used when navigating to a new url or updating history, + * but is not needed when simply generating a string for a link. + */ + +export function encode_state(state, sidebar, with_root) { + const path = []; + if (state.host && (sidebar || state.host !== "localhost")) + path.push("@" + state.host); + if (state.component) + path.push.apply(path, state.component.split("/")); + let string = cockpit.location.encode(path, null, with_root); + if (state.hash && state.hash !== "/") + string += "#" + state.hash; + return string; +} + +/* Decodes navigate state from a string */ +export function decode_state(string) { + const state = { version: "v1", hash: "" }; + const pos = string.indexOf("#"); + if (pos !== -1) { + state.hash = string.substring(pos + 1); + string = string.substring(0, pos); + } + if (string[0] != '/') + string = "/" + string; + const path = cockpit.location.decode(string); + if (path[0] && path[0][0] == "@") { + state.host = path.shift().substring(1); + state.sidebar = true; + } else { + state.host = "localhost"; + } + if (path.length && path[path.length - 1] == "index") + path.pop(); + state.component = path.join("/"); + return state; +} + +/* Build an href for use in an */ +export function build_href(state, sidebar) { + if (sidebar == undefined) + sidebar = true; + return encode_state(state, sidebar); +} + +export function decode_window_location() { + const shell_embedded = window.location.pathname.indexOf(".html") !== -1; + + if (shell_embedded) + return decode_state("/" + window.location.hash); + else + return decode_state(window.location.pathname + window.location.hash); +} + +export function encode_location(location) { + const shell_embedded = window.location.pathname.indexOf(".html") !== -1; + return shell_embedded ? window.location : encode_state(location, null, true); +} + +export function replace_window_location(location) { + window.history.replaceState(location, "", encode_location(location)); +}