Skip to content

Commit

Permalink
Add url field to routes to build path and use it everywhere (#1035)
Browse files Browse the repository at this point in the history
Will fix #844 

I considered multiple approaches to this. I think having an `url` field
on the route object itself is the nicest, at least for using it.
`DirectVideoRoute.url({ videoid: ... })` reads very clearly to me. But
it wasn't that straight forward to find a way that plays nicely with
Typescript.

Adding the `url` field to `Route` and keeping every `FooRoute` as type
`Route` is tricky because (a) in the beginning, not every route will
have an `url` field, and (b) the url builder has different arguments per
route, making it generic and more complicated. We certainly want to
involve `Route` somehow such that the `match` function is well typed
without the need to annotate the type of its argument. So yeah, this is
what I came up with.

- Is this a good approach?
- Is `url` even the right term? It always just returns a path. Which is
an URL... or URI, I can never tell.

Also: hide whitespace changes in githubs diff view when looking at this!

Obviously this is not nearly done, just a tiny proof of concept to get
feedback. Which I'd like to get from @JulianKniephoff as well.
  • Loading branch information
owi92 authored Jan 15, 2024
2 parents ce691ff + 12adaef commit 4b7bf80
Show file tree
Hide file tree
Showing 26 changed files with 603 additions and 518 deletions.
4 changes: 2 additions & 2 deletions frontend/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import ReactDOM from "react-dom/client";
import { App } from "./App";
import "./i18n";
import { matchInitialRoute } from "./router";
import { REDIRECT_STORAGE_KEY } from "./routes/Login";
import { LoginRoute, REDIRECT_STORAGE_KEY } from "./routes/Login";


// Potentially redirect to previous page after login.
Expand All @@ -19,7 +19,7 @@ import { REDIRECT_STORAGE_KEY } from "./routes/Login";
// a component to trigger a redirect. In fact, that approach would break in
// `StrictMode`.
const redirectTo = window.sessionStorage.getItem(REDIRECT_STORAGE_KEY);
if (window.location.pathname === "/~login" && redirectTo) {
if (window.location.pathname === LoginRoute.url && redirectTo) {
const newUri = new URL(redirectTo, document.baseURI).href;
// eslint-disable-next-line no-console
console.debug(`Requested login page after login: redirecting to previous page ${newUri}`);
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/layout/Footer.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { useTranslation } from "react-i18next";
import CONFIG from "../config";
import { Link } from "../router";
import { ABOUT_PATH } from "../routes/paths";
import { translatedConfig } from "../util";
import { COLORS } from "../color";
import { AboutRoute } from "../routes/About";


export const Footer: React.FC = () => {
Expand Down Expand Up @@ -36,7 +36,7 @@ export const Footer: React.FC = () => {
{CONFIG.footerLinks.map((entry, i) => {
if (entry === "about") {
return <li key={i}>
<Link to={ABOUT_PATH}>{t("footer.about-tobira")}</Link>
<Link to={AboutRoute.url}>{t("footer.about-tobira")}</Link>
</li>;
} else if (entry === "graphiql") {
return <li key={i}>
Expand Down
9 changes: 4 additions & 5 deletions frontend/src/layout/header/Search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next";
import { HiOutlineSearch } from "react-icons/hi";
import { LuX } from "react-icons/lu";
import { useRouter } from "../../router";
import { handleNavigation, isSearchActive } from "../../routes/Search";
import { handleNavigation, SearchRoute, isSearchActive } from "../../routes/Search";
import { focusStyle } from "../../ui";
import { Spinner } from "../../ui/Spinner";
import { currentRef } from "../../util";
Expand Down Expand Up @@ -61,14 +61,13 @@ export const SearchField: React.FC<SearchFieldProps> = ({ variant }) => {
const lastTimeout = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
useEffect(() => () => clearTimeout(lastTimeout.current));

const defaultValue = isSearchActive()
const onSearchRoute = isSearchActive();
const defaultValue = onSearchRoute
? new URLSearchParams(document.location.search).get("q") ?? undefined
: undefined;

const onSearchRoute = document.location.pathname === "/~search";
const search = (expression: string) => {
const newUrl = `/~search?q=${encodeURIComponent(expression)}`;
router.goto(newUrl, onSearchRoute);
router.goto(SearchRoute.url({ query: expression }), onSearchRoute);
};

return (
Expand Down
14 changes: 8 additions & 6 deletions frontend/src/layout/header/UserBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ import { User, useUser } from "../../User";
import { ActionIcon, ICON_STYLE } from "./ui";
import CONFIG from "../../config";
import { Spinner } from "../../ui/Spinner";
import { LOGIN_PATH } from "../../routes/paths";
import { REDIRECT_STORAGE_KEY } from "../../routes/Login";
import { LoginRoute, REDIRECT_STORAGE_KEY } from "../../routes/Login";
import { focusStyle } from "../../ui";
import { ExternalLink } from "../../relay/auth";
import { COLORS } from "../../color";
import { translatedConfig } from "../../util";
import { UploadRoute } from "../../routes/Upload";
import { ManageRoute } from "../../routes/manage";
import { ManageVideosRoute } from "../../routes/manage/Video";



Expand Down Expand Up @@ -64,7 +66,7 @@ const LoggedOut: React.FC = () => {

return (
<Link
to={CONFIG.auth.loginLink ?? LOGIN_PATH}
to={CONFIG.auth.loginLink ?? LoginRoute.url}
onClick={() => {
// Store a redirect link in session storage.
window.sessionStorage.setItem(REDIRECT_STORAGE_KEY, window.location.href);
Expand Down Expand Up @@ -178,7 +180,7 @@ const LoggedIn: React.FC<LoggedInProps> = ({ user }) => {
const items: HeaderMenuProps["items"] = [
{
icon: <LuFolder />,
wrapper: <Link to="/~manage" />,
wrapper: <Link to={ManageRoute.url} />,
children: t("user.manage-content"),
css: { minWidth: 200 },
},
Expand All @@ -190,13 +192,13 @@ const LoggedIn: React.FC<LoggedInProps> = ({ user }) => {
}] : [],
{
icon: <LuFilm />,
wrapper: <Link to="/~manage/videos" />,
wrapper: <Link to={ManageVideosRoute.url} />,
children: t("manage.my-videos.title"),
css: indent,
},
...user.canUpload ? [{
icon: <LuUpload />,
wrapper: <Link to="/~manage/upload" />,
wrapper: <Link to={UploadRoute.url} />,
children: t("upload.title"),
css: indent,
}] : [],
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/rauta.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ export type MatchedRoute = {
dispose?: () => void;
};

/** Creates the internal representation of the given route. */
export const makeRoute = (match: (url: URL) => MatchedRoute | null): Route => ({ match });
// /** Creates the internal representation of the given route. */
// export const makeRoute = (match: (url: URL) => MatchedRoute | null): Route => ({ match });

/** Will replace `makeRoute` in the future. */
export const makeRoute = <T extends Route>(obj: T): T => obj;

/** Routing definition */
interface Config {
Expand Down
31 changes: 17 additions & 14 deletions frontend/src/routes/About.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,30 @@ import { Nav } from "../layout/Navigation";
import { RootLoader } from "../layout/Root";
import { loadQuery } from "../relay";
import { AboutQuery } from "./__generated__/AboutQuery.graphql";
import { ABOUT_PATH } from "./paths";
import { makeRoute } from "../rauta";
import { PageTitle } from "../layout/header/ui";
import { Breadcrumbs } from "../ui/Breadcrumbs";
import { COLORS } from "../color";

const PATH = "/~tobira";

export const AboutRoute = makeRoute(url => {
if (url.pathname !== ABOUT_PATH) {
return null;
}
export const AboutRoute = makeRoute({
url: PATH,
match: url => {
if (url.pathname !== PATH) {
return null;
}

const queryRef = loadQuery<AboutQuery>(query, {});
return {
render: () => <RootLoader
{...{ query, queryRef }}
nav={data => <Nav fragRef={data.realm} />}
render={() => <About />}
/>,
dispose: () => queryRef.dispose(),
};
const queryRef = loadQuery<AboutQuery>(query, {});
return {
render: () => <RootLoader
{...{ query, queryRef }}
nav={data => <Nav fragRef={data.realm} />}
render={() => <About />}
/>,
dispose: () => queryRef.dispose(),
};
},
});

const query = graphql`
Expand Down
97 changes: 51 additions & 46 deletions frontend/src/routes/Embed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Translation, useTranslation } from "react-i18next";
import { graphql, PreloadedQuery, usePreloadedQuery } from "react-relay";
import { unreachable } from "@opencast/appkit";

import { eventId, isSynced } from "../util";
import { eventId, isSynced, keyOfId } from "../util";
import { GlobalErrorBoundary } from "../util/err";
import { loadQuery } from "../relay";
import { makeRoute } from "../rauta";
Expand Down Expand Up @@ -39,34 +39,37 @@ const query = graphql`
}
`;

export const EmbedVideoRoute = makeRoute(url => {
const regex = new RegExp(`^/~embed/!v/(${b64regex}+)/?$`, "u");
const params = regex.exec(url.pathname);
if (params === null) {
return null;
}
const videoId = decodeURIComponent(params[1]);

const queryRef = loadQuery<EmbedQuery>(query, { id: eventId(videoId) });

return {
render: () => <ErrorBoundary>
<Suspense fallback={
<PlayerPlaceholder>
<Spinner css={{
"& > circle": {
stroke: "white",
},
}} />
</PlayerPlaceholder>
}>
<PlayerContextProvider>
<Embed queryRef={queryRef} />
</PlayerContextProvider>
</Suspense>
</ErrorBoundary>,
dispose: () => queryRef.dispose(),
};
export const EmbedVideoRoute = makeRoute({
url: ({ videoId }: { videoId: string }) => `/~embed/!v/${keyOfId(videoId)}`,
match: url => {
const regex = new RegExp(`^/~embed/!v/(${b64regex}+)/?$`, "u");
const params = regex.exec(url.pathname);
if (params === null) {
return null;
}
const videoId = decodeURIComponent(params[1]);

const queryRef = loadQuery<EmbedQuery>(query, { id: eventId(videoId) });

return {
render: () => <ErrorBoundary>
<Suspense fallback={
<PlayerPlaceholder>
<Spinner css={{
"& > circle": {
stroke: "white",
},
}} />
</PlayerPlaceholder>
}>
<PlayerContextProvider>
<Embed queryRef={queryRef} />
</PlayerContextProvider>
</Suspense>
</ErrorBoundary>,
dispose: () => queryRef.dispose(),
};
},
});

type EmbedProps = {
Expand Down Expand Up @@ -105,25 +108,27 @@ const Embed: React.FC<EmbedProps> = ({ queryRef }) => {
return <Player event={event} />;
};

export const BlockEmbedRoute = makeRoute(url => {
// Only do something if we are embedded
if (window === window.top) {
return null;
}
export const BlockEmbedRoute = makeRoute({
match: url => {
// Only do something if we are embedded
if (window === window.top) {
return null;
}

// And only if this is a non-embeddable route
if (url.pathname.startsWith("/~embed/")) {
return null;
}
// And only if this is a non-embeddable route
if (url.pathname.startsWith("/~embed/")) {
return null;
}

return {
render: () => <PlayerPlaceholder>
<LuAlertTriangle />
<div>
<Translation>{t => t("embed.not-supported")}</Translation>
</div>
</PlayerPlaceholder>,
};
return {
render: () => <PlayerPlaceholder>
<LuAlertTriangle />
<div>
<Translation>{t => t("embed.not-supported")}</Translation>
</div>
</PlayerPlaceholder>,
};
},
});

class ErrorBoundary extends GlobalErrorBoundary {
Expand Down
34 changes: 18 additions & 16 deletions frontend/src/routes/InvalidUrl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,25 @@ import { ErrorPage } from "../ui/error";
import { InvalidUrlQuery } from "./__generated__/InvalidUrlQuery.graphql";


export const InvalidUrlRoute = makeRoute(url => {
try {
decodeURIComponent(url.pathname);
} catch (e) {
if (e instanceof URIError) {
const queryRef = loadQuery<InvalidUrlQuery>(query, {});
return {
render: () => <RootLoader
{...{ query, queryRef }}
nav={() => []}
render={() => <InvalidUrl />}
/>,
dispose: () => queryRef.dispose(),
};
export const InvalidUrlRoute = makeRoute({
match: url => {
try {
decodeURIComponent(url.pathname);
} catch (e) {
if (e instanceof URIError) {
const queryRef = loadQuery<InvalidUrlQuery>(query, {});
return {
render: () => <RootLoader
{...{ query, queryRef }}
nav={() => []}
render={() => <InvalidUrl />}
/>,
dispose: () => queryRef.dispose(),
};
}
}
}
return null;
return null;
},
});

const query = graphql`
Expand Down
23 changes: 13 additions & 10 deletions frontend/src/routes/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { Spinner } from "../ui/Spinner";
import { LuCheck, LuChevronLeft, LuLogIn } from "react-icons/lu";
import { Card } from "../ui/Card";
import CONFIG from "../config";
import { LOGIN_PATH } from "./paths";
import { makeRoute } from "../rauta";
import { Header } from "../layout/header";
import { BREAKPOINT_MEDIUM } from "../GlobalStyle";
Expand All @@ -28,17 +27,21 @@ import { focusStyle } from "../ui";


export const REDIRECT_STORAGE_KEY = "tobira-redirect-after-login";
const PATH = "/~login";

export const LoginRoute = makeRoute(url => {
if (url.pathname !== LOGIN_PATH) {
return null;
}
export const LoginRoute = makeRoute({
url: PATH,
match: url => {
if (url.pathname !== PATH) {
return null;
}

const queryRef = loadQuery<LoginQuery>(query, {});
return {
render: () => <Login queryRef={queryRef} />,
dispose: () => queryRef.dispose(),
};
const queryRef = loadQuery<LoginQuery>(query, {});
return {
render: () => <Login queryRef={queryRef} />,
dispose: () => queryRef.dispose(),
};
},
});

const query = graphql`
Expand Down
Loading

0 comments on commit 4b7bf80

Please sign in to comment.