Skip to content

Commit

Permalink
WIP - navigation experiment
Browse files Browse the repository at this point in the history 
let's git rid of our state objects (but keep pushState and
replaceState in the right places).
  • Loading branch information
@mvollmer
mvollmer committed Sep 23, 2024
1 parent 24b1266 commit 2d3f8aa
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 93 deletions.
91 changes: 20 additions & 71 deletions pkg/shell/base_index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
};
Expand All @@ -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();

Expand All @@ -168,18 +121,18 @@ 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);
const target = shell_embedded ? window.location : encode_state(state, null, true);

if (replace) {
console.log("REPLACE", JSON.stringify(state), target);
history.replaceState(state, "", target);
return false;
}

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)
Expand All @@ -191,11 +144,6 @@ function Index() {
return false;
};

/* Build an href for use in an <a> */
self.href = function (state, sidebar) {
return encode(state, sidebar);
};

self.show_oops = function () {
self.has_oops = true;
self.dispatchEvent("update");
Expand All @@ -220,6 +168,7 @@ function Index() {

self.ready = function () {
window.addEventListener("popstate", ev => {
console.log("POP!");
self.navigate(ev.state, true);
});

Expand Down
12 changes: 5 additions & 7 deletions pkg/shell/hosts.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -141,7 +141,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);
Expand Down Expand Up @@ -196,7 +196,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);
}
}
Expand All @@ -210,7 +210,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);
}

Expand Down Expand Up @@ -241,7 +241,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"),
Expand All @@ -250,7 +249,7 @@ export class CockpitHosts extends React.Component {
const render = (m, term) => <CockpitNavItem
term={term}
keyword={m.keyword}
to={hostAddr({ host: m.address }, true)}
to={build_href({ host: m.address })}
active={m === this.props.machine}
key={m.key}
name={m.label}
Expand Down Expand Up @@ -336,6 +335,5 @@ CockpitHosts.propTypes = {
index: PropTypes.object.isRequired,
loader: PropTypes.object.isRequired,
selector: PropTypes.string.isRequired,
hostAddr: PropTypes.func.isRequired,
jump: PropTypes.func.isRequired,
};
7 changes: 4 additions & 3 deletions pkg/shell/nav.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { SearchInput } from "@patternfly/react-core/dist/esm/components/SearchIn
import { Tooltip, TooltipPosition } from "@patternfly/react-core/dist/esm/components/Tooltip/index.js";
import { ContainerNodeIcon, ExclamationCircleIcon, ExclamationTriangleIcon, InfoCircleIcon } from '@patternfly/react-icons';

import { build_href } from "./util.jsx";

const _ = cockpit.gettext;

export const SidebarToggle = () => {
Expand Down Expand Up @@ -253,7 +255,6 @@ export const PageNav = ({
component,
compiled,
page_status,
build_href,
jump
}) => {
if (!machine || machine.state != "connected") {
Expand Down Expand Up @@ -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} />
);
}
Expand All @@ -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 <CockpitNav groups={groups}
Expand Down
8 changes: 6 additions & 2 deletions pkg/shell/router.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

import cockpit from "cockpit";

import { encode_state } from "./util.jsx";

export function Router(index) {
const self = this;

Expand Down Expand Up @@ -99,10 +101,12 @@ export function Router(index) {
if (hash === "/")
hash = "";
/* The browser has already pushed an appropriate entry to
the history, so let's just replace it with our custom
state object.
the history, so let's just replace it with one that
includes the right hash.
*/
const state = Object.assign({}, index.retrieve_state(), { hash });
window.history.replaceState(state, "", encode_state(state, null, true));
index.track_hash(state.host, state.component, state.hash);
index.navigate(state, true);
}
}
Expand Down
10 changes: 4 additions & 6 deletions pkg/shell/shell.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ const _ = cockpit.gettext;

// TODO:

/* Split out router from base_index.
/* Move hash tracking from Router to Frames
*/

/* Get rid of base_index.current_frame. In fact, move all of base_index into ShellState.
/* Get rid of "state.sidebar". What is that even about?
*/

/* Do something about jump calling navigate calling jump.
/* Get rid of base_index.current_frame. In fact, move all of base_index into ShellState.
*/

/* Use Dialogs.run for HostsDialog, get rid of trigger_connection_flow
Expand Down Expand Up @@ -128,7 +128,7 @@ const Shell = () => {
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];
Expand Down Expand Up @@ -177,7 +177,6 @@ const Shell = () => {
component={component}
compiled={compiled}
page_status={page_status}
build_href={state.index.href}
jump={state.index.jump} />
</nav>
</div>
Expand All @@ -189,7 +188,6 @@ const Shell = () => {
index={state.index}
loader={state.loader}
selector="nav-hosts"
hostAddr={state.index.href}
jump={state.index.jump} />
: <CockpitCurrentHost current_user={current_user} machine={machine} />
}
Expand Down
15 changes: 11 additions & 4 deletions pkg/shell/state.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import cockpit from "cockpit";

import { machines as machines_factory } from "./machines/machines.js";
import { encode_state } from "./util.jsx";

import * as base_index from "./base_index";

/* XXX - This is a hacked up version of the old indexes.jsx, without
Expand Down Expand Up @@ -345,9 +347,14 @@ export function ShellState(trigger_connection_flow) {
if (self.problem)
return;

console.log("passed state", JSON.stringify(state));

state = null;
if (!state)

Check warning

Code scanning / CodeQL

Useless conditional Warning

This negation always evaluates to true.
state = index.retrieve_state();

console.log("NAV", JSON.stringify(state), 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
Expand Down Expand Up @@ -379,8 +386,11 @@ export function ShellState(trigger_connection_flow) {
}

const compiled = compile(machine);
if (machine.manifests && !state.component)
if (machine.manifests && !state.component) {
state.component = choose_component(state, compiled);
console.log("REPLACE with choosen");
window.history.replaceState(state, "", encode_state(state, null, true));
}

self.machine = machine;
self.fullpath = state.component;
Expand All @@ -390,9 +400,6 @@ export function ShellState(trigger_connection_flow) {

update_frame(machine, state, compiled.items[self.component]?.label);

/* Just replace the state, and URL */
index.jump(state, true);

update();
}

Expand Down

0 comments on commit 2d3f8aa

Please sign in to comment.