Skip to content

Rationalise URLs #970

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/base.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const baseUrl = (() => {
let base = process.env.PUBLIC_URL || "/";
if (!base.endsWith("/")) {
base += "/";
}
return base;
})();
9 changes: 2 additions & 7 deletions src/documentation/api/ApiDocumentation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,7 @@ import { docStyles } from "../../common/documentation-styles";
import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
import { splitDocString } from "../../editor/codemirror/language-server/docstrings";
import { ApiDocsEntry, ApiDocsResponse } from "../../language-server/apidocs";
import {
Anchor,
RouterParam,
useRouterParam,
useRouterState,
} from "../../router-hooks";
import { Anchor, useRouterTabSlug, useRouterState } from "../../router-hooks";
import DocString from "../common/DocString";
import { useAnimationDirection } from "../common/documentation-animation-hooks";
import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
Expand All @@ -32,7 +27,7 @@ interface ApiDocumentationProps {
}

export const ApiDocumentation = ({ docs }: ApiDocumentationProps) => {
const [anchor, setAnchor] = useRouterParam(RouterParam.api);
const [anchor, setAnchor] = useRouterTabSlug("api");
const handleNavigate = useCallback(
(id: string | undefined) => {
setAnchor(id ? { id } : undefined, "documentation-user");
Expand Down
68 changes: 1 addition & 67 deletions src/documentation/api/apidocs-util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@
* SPDX-License-Identifier: MIT
*/
import { ApiDocsResponse } from "../../language-server/apidocs";
import {
moduleAndApiFromId,
pullModulesToTop,
resolveDottedName,
} from "./apidocs-util";
import { moduleAndApiFromId, pullModulesToTop } from "./apidocs-util";

describe("pullModulesToTop", () => {
it("pulls modules up", () => {
Expand Down Expand Up @@ -72,68 +68,6 @@ describe("pullModulesToTop", () => {
});
});

describe("resolveDottedName", () => {
const docs: ApiDocsResponse = {
microbit: {
fullName: "microbit",
id: "microbit",
name: "microbit",
kind: "module",
children: [
{
fullName: "microbit.Button",
name: "Button",
kind: "class",
id: "microbit.Button",
children: [
{
fullName: "microbit.Button.is_pressed",
kind: "function",
id: "microbit.Button.is_pressed",
name: "is_pressed",
},
],
},
],
},
"microbit.compass": {
fullName: "microbit.compass",
id: "microbit.compass",
name: "compass",
kind: "module",
children: [
{
fullName: "microbit.compass.get_x",
kind: "function",
id: "microbit.compass.get_x",
name: "get_x",
},
],
},
};
it("finds modules", () => {
expect(resolveDottedName(docs, "microbit.compass")).toEqual(
docs["microbit.compass"]
);
expect(resolveDottedName(docs, "microbit")).toEqual(docs["microbit"]);
});
it("finds classes", () => {
expect(resolveDottedName(docs, "microbit.Button")?.fullName).toEqual(
"microbit.Button"
);
});
it("finds functions", () => {
expect(resolveDottedName(docs, "microbit.compass.get_x")?.fullName).toEqual(
"microbit.compass.get_x"
);
});
it("finds class members", () => {
expect(
resolveDottedName(docs, "microbit.Button.is_pressed")?.fullName
).toEqual("microbit.Button.is_pressed");
});
});

describe("moduleAndApiFromId", () => {
it("correctly splits module and apiId from id with three segments", () => {
const id = "microbit.display.scroll";
Expand Down
18 changes: 0 additions & 18 deletions src/documentation/api/apidocs-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,6 @@ export const resolveModule = (
);
};

export const resolveDottedName = (docs: ApiDocsResponse, name: string) => {
let entry = resolveModule(docs, name);
if (!entry) {
return undefined;
}
const remainder = name.substring(entry.fullName.length + 1);
if (remainder.length > 0) {
for (const part of remainder.split(".")) {
if (entry && entry.children) {
entry = entry.children.find((e) => e.name === part);
} else {
return undefined;
}
}
}
return entry;
};

export const moduleAndApiFromId = (id: string) => {
const idSegments = id.split(".");
const pythonModuleName = idSegments[0];
Expand Down
4 changes: 2 additions & 2 deletions src/documentation/common/DocumentationContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ const DocumentationApiLinkMark = (
e.preventDefault();
setState({
tab: "api",
api: { id: props.mark.name },
slug: { id: props.mark.name },
});
}}
>
Expand All @@ -96,7 +96,7 @@ const DocumentationInternalLinkMark = (
{
...state,
tab: "reference",
reference: {
slug: {
id: props.mark.slug.current,
},
},
Expand Down
4 changes: 2 additions & 2 deletions src/documentation/ideas/IdeasDocumentation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { docStyles } from "../../common/documentation-styles";
import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
import { getAspectRatio, imageUrlBuilder } from "../../common/imageUrlBuilder";
import { useResizeObserverContentRect } from "../../common/use-resize-observer";
import { Anchor, RouterParam, useRouterParam } from "../../router-hooks";
import { Anchor, useRouterTabSlug } from "../../router-hooks";
import { useAnimationDirection } from "../common/documentation-animation-hooks";
import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
import DocumentationContent, {
Expand All @@ -27,7 +27,7 @@ interface IdeasDocumentationProps {
}

const IdeasDocumentation = ({ ideas }: IdeasDocumentationProps) => {
const [anchor, setAnchor] = useRouterParam(RouterParam.idea);
const [anchor, setAnchor] = useRouterTabSlug("ideas");
const direction = useAnimationDirection(anchor);
const ideaId = anchor?.id;
const handleNavigate = useCallback(
Expand Down
4 changes: 2 additions & 2 deletions src/documentation/reference/ReferenceDocumentation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { useCallback } from "react";
import { useIntl } from "react-intl";
import { docStyles } from "../../common/documentation-styles";
import HeadedScrollablePanel from "../../common/HeadedScrollablePanel";
import { Anchor, RouterParam, useRouterParam } from "../../router-hooks";
import { Anchor, useRouterTabSlug } from "../../router-hooks";
import { useAnimationDirection } from "../common/documentation-animation-hooks";
import DocumentationBreadcrumbHeading from "../common/DocumentationBreadcrumbHeading";
import DocumentationContent from "../common/DocumentationContent";
Expand All @@ -29,7 +29,7 @@ interface ReferenceDocumentationProps {
* generate the API documentation.
*/
const ReferenceToolkit = ({ toolkit }: ReferenceDocumentationProps) => {
const [anchor, setAnchor] = useRouterParam(RouterParam.reference);
const [anchor, setAnchor] = useRouterTabSlug("reference");
const direction = useAnimationDirection(anchor);
const topicOrEntryId = anchor?.id.split("/")[0];
const handleNavigate = useCallback(
Expand Down
2 changes: 1 addition & 1 deletion src/documentation/search/search.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe("Search", () => {
id: "indentations",
navigation: {
tab: "reference",
reference: { id: "indentations" },
slug: { id: "indentations" },
},
extract: {
title: [
Expand Down
2 changes: 1 addition & 1 deletion src/documentation/search/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class SearchIndex {
containerTitle: content.containerTitle,
navigation: {
tab: this.tab,
[this.tab]: { id: content.id },
slug: { id: content.id },
},
extract: extracts,
};
Expand Down
2 changes: 1 addition & 1 deletion src/editor/codemirror/CodeMirror.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ const CodeMirror = ({
setRouterState(
{
tab,
[tab]: { id },
slug: { id },
},
"documentation-from-code"
);
Expand Down
1 change: 1 addition & 0 deletions src/editor/codemirror/language-server/documentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export const wrapWithDocumentationButton = (
apiAnchor.textContent = intl.formatMessage({ id: "api-tab" });
apiAnchor.onclick = (e) => {
e.preventDefault();
// Could we instead interact with the history API here?
document.dispatchEvent(
new CustomEvent("cm/openDocs", {
detail: {
Expand Down
5 changes: 2 additions & 3 deletions src/language-server/pyright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
BrowserMessageReader,
BrowserMessageWriter,
} from "vscode-jsonrpc/browser";
import { baseUrl } from "../base";
import { createUri, LanguageServerClient } from "./client";

// This is modified by bin/update-pyright.sh
Expand All @@ -24,9 +25,7 @@ export const pyright = (language: string): LanguageServerClient | undefined => {
return undefined;
}
// Needed to support review branches that use a path location.
const { origin, pathname } = window.location;
const base = `${origin}${pathname}${pathname.endsWith("/") ? "" : "/"}`;
const workerScript = `${base}workers/${workerScriptName}`;
const workerScript = `${baseUrl}workers/${workerScriptName}`;
const foreground = new Worker(workerScript, {
name: "Pyright-foreground",
});
Expand Down
93 changes: 37 additions & 56 deletions src/router-hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ import {
useMemo,
useState,
} from "react";
import { baseUrl } from "./base";
import { useLogging } from "./logging/logging-hooks";

export type TabName = "api" | "ideas" | "reference" | "project";

/**
* An anchor-like navigation used for scroll positions.
*
Expand All @@ -33,25 +36,9 @@ export interface Anchor {
const anchorForParam = (param: string | null): Anchor | undefined =>
param ? { id: param } : undefined;

export class RouterParam<T> {
static tab: RouterParam<string> = new RouterParam("tab");
static api: RouterParam<Anchor> = new RouterParam("api");
static reference: RouterParam<Anchor> = new RouterParam("reference");
static idea: RouterParam<Anchor> = new RouterParam("idea");

private constructor(public id: keyof RouterState) {}

get(state: RouterState): T | undefined {
// Constructor is private so this is safe.
return state[this.id] as unknown as T | undefined;
}
}

export interface RouterState {
tab?: string;
reference?: Anchor;
api?: Anchor;
idea?: Anchor;
tab?: TabName;
slug?: Anchor;
focus?: boolean;
}

Expand All @@ -68,14 +55,21 @@ type RouterContextValue = [

const RouterContext = createContext<RouterContextValue | undefined>(undefined);

const parse = (search: string): RouterState => {
const params = new URLSearchParams(search);
return {
tab: params.get("tab") ?? undefined,
api: anchorForParam(params.get("api")),
reference: anchorForParam(params.get("reference")),
idea: anchorForParam(params.get("idea")),
};
const parse = (pathname: string): RouterState => {
pathname = pathname.slice(baseUrl.length);
if (pathname) {
const parts = pathname.split("/");
const tab = parts[0];
if (
tab === "api" ||
tab === "reference" ||
tab === "ideas" ||
tab === "project"
) {
return { tab, slug: anchorForParam(parts[1]) };
}
}
return {};
};

/**
Expand All @@ -95,28 +89,19 @@ export const useRouterState = (): RouterContextValue => {
};

export const toUrl = (state: RouterState): string => {
const query = Object.entries(state)
.filter(([k, v]) => k !== "focus" && !!v)
.map(([k, v]) => {
return `${encodeURIComponent(k)}=${encodeURIComponent(
serializeValue(v)
)}`;
})
.join("&");
return window.location.toString().split("?")[0] + (query ? "?" + query : "");
const parts = [state.tab, state.slug?.id];
const pathname = baseUrl + parts.filter((x): x is string => !!x).join("/");
return window.location.toString().split("/", 1)[0] + pathname;
};

const serializeValue = (value: Anchor | string) =>
typeof value === "string" ? value : value.id;

export const RouterProvider = ({ children }: { children: ReactNode }) => {
const logging = useLogging();
const [state, setState] = useState(parse(window.location.search));
const [state, setState] = useState(parse(window.location.pathname));
useEffect(() => {
// This detects browser navigation but not our programatic changes,
// so we need to update state there ourselves.
const listener = (_: PopStateEvent) => {
const newState = parse(window.location.search);
const newState = parse(window.location.pathname);
setState(newState);
};
window.addEventListener("popstate", listener);
Expand All @@ -129,10 +114,7 @@ export const RouterProvider = ({ children }: { children: ReactNode }) => {
if (source) {
logging.event({
type: source,
message:
(newState.reference?.id && `reference-${newState.reference?.id}`) ||
(newState.api?.id && `api-${newState.api?.id}`) ||
(newState.idea?.id && `ideas-${newState.idea?.id}`),
message: toUrl(newState),
});
}
const url = toUrl(newState);
Expand All @@ -151,24 +133,23 @@ export const RouterProvider = ({ children }: { children: ReactNode }) => {
};

/**
* Access a single parameter of the router state.
* All other parameters will remain unchanged if you set this parameter.
* Access the slug for a particular tab.
*
* @param param The parameter name.
* @returns A [state, setState] pair for the parameter.
* @param tab The tab name.
* @returns A [state, setState] pair for the tab.
*/
export const useRouterParam = <T,>(
param: RouterParam<T>
export const useRouterTabSlug = (
tab: TabName
): [
T | undefined,
(param: T | undefined, source?: NavigationSource) => void
Anchor | undefined,
(param: Anchor | undefined, source?: NavigationSource) => void
] => {
const [state, setState] = useRouterState();
const navigateParam = useCallback(
(value: T | undefined, source?: NavigationSource) => {
setState({ ...state, [param.id]: value }, source);
(value: Anchor | undefined, source?: NavigationSource) => {
setState({ ...state, tab, slug: value }, source);
},
[param, setState, state]
[tab, setState, state]
);
return [param.get(state), navigateParam];
return [state.tab === tab ? state.slug : undefined, navigateParam];
};
Loading