Skip to content

Commit

Permalink
Erase the URL query on login unless the user opts in.
Browse files Browse the repository at this point in the history
Users often open Votr with a saved link from their bookmarks or history. If the
link is bad (e.g. it has an invalid key for some reason), the user gets an error
every time they log in with it. Even for valid URLs, I'm not so sure whether the
user really wanted to visit that page or it's just a waste of resources.

The login page now has a checkbox to respect the current URL. It is false by
default, but it is persisted in localStorage. I hope this will be better for
users who use saved links by accident, while still allowing power users to
bookmark pages if they want.

Stats for 2023-01:
- 71% of logins had empty destination
- 21% of logins only had ?action= in destination
- 8% of logins had multiple query parameters

Updates #65. Updates #133.
  • Loading branch information
TomiBelan committed Feb 19, 2023
1 parent 66ed2d6 commit 61a9bb5
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 8 deletions.
36 changes: 33 additions & 3 deletions votrfront/js/LoginPage.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import _ from "lodash";
import React, { useState } from "react";
import React, { useContext, useState } from "react";
import { AboutModal } from "./About";
import { actionTitles } from "./actions";
import { Modal, ModalBase } from "./layout";
import { FakeLink } from "./router";
import { getLocalSetting, setLocalSetting } from "./LocalSettings";
import { buildUrl, FakeLink, QueryContext } from "./router";

var TYPE_NAMES: Record<string, string> = {
"cosignproxy": "Cosign (automatické)",
Expand All @@ -12,6 +14,34 @@ var TYPE_NAMES: Record<string, string> = {
"demo": "Demo",
};

function DestinationCheckbox() {
var query = useContext(QueryContext);
var { action, modal } = query;
var actionTitle = action && actionTitles[action];

// Don't show the checkbox if the query string is empty or it only contains
// foreign params (?fbclid=...).
if (!action && !modal) return null;

return (
<p>
<label>
<input
type="checkbox"
name="destination"
value={buildUrl(query)}
checked={getLocalSetting("useDestination") == "true"}
onChange={(event) => {
setLocalSetting("useDestination", String(event.target.checked));
}}
/>{" "}
Začať tam kde predtým {actionTitle ? "(" + actionTitle + ") " : null}
podľa uloženej webovej adresy
</label>
</p>
);
}

function LoginForm({ onOpenError }: { onOpenError: () => void }) {
var [state, setState] = useState({
server: Votr.settings.server || 0,
Expand Down Expand Up @@ -62,7 +92,7 @@ function LoginForm({ onOpenError }: { onOpenError: () => void }) {
</React.Fragment>
)}

<input type="hidden" name="destination" value={location.search} />
<DestinationCheckbox />

{Votr.settings.servers!.length > 1 ? (
<p>
Expand Down
19 changes: 16 additions & 3 deletions votrfront/js/actions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ function makeNotFoundPage() {
}

var actions: Record<string, () => React.ReactNode> = {
index: makeIndexPage,
priebezneHodnotenia: makePriebezneHodnoteniaPage,
mojeHodnotenia: makeMojeHodnoteniaPage,
mojePredmety: makeMojePredmetyPage,
Expand All @@ -37,6 +36,20 @@ var actions: Record<string, () => React.ReactNode> = {
zapisZPonuky: makeZapisZPonukyPage,
};

// TODO: Reduce code duplication of page titles.
// TODO: Set document.title immediately even if a ...Selector is loading.
export const actionTitles: Record<string, string> = {
priebezneHodnotenia: "Priebežné hodnotenia",
mojeHodnotenia: "Moje hodnotenia",
mojePredmety: "Moje predmety",
mojeSkusky: "Moje skúšky",
prehladStudia: "Prehľad štúdia",
registerOsob: "Register osôb",
registerPredmetov: "Register predmetov",
zapisZPlanu: "Zápis predmetov",
zapisZPonuky: "Zápis predmetov",
};

var modalActions: Record<string, React.ComponentType> = {
about: AboutModal,
detailPredmetu: DetailPredmetuModal,
Expand All @@ -45,8 +58,8 @@ var modalActions: Record<string, React.ComponentType> = {

export function App() {
var query = useContext(QueryContext);
var action = query.action || "index";
var maker = actions[action] || makeNotFoundPage;
var action = query.action;
var maker = action ? actions[action] || makeNotFoundPage : makeIndexPage;
var modalComponent = Votr.ajaxError
? ErrorModal
: query.modal
Expand Down
2 changes: 1 addition & 1 deletion votrfront/js/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function Root({ app }: { app: React.ComponentType }) {
);
}

function buildUrl(href: string | Href) {
export function buildUrl(href: string | Href) {
if (_.isString(href)) return href;
return "?" + $.param(_.omitBy(href, _.isUndefined), true);
}
Expand Down
2 changes: 1 addition & 1 deletion votrfront/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def start_login(request, destination, params):

def login(request):
params = request.values.to_dict()
return start_login(request, params.pop('destination'), params)
return start_login(request, params.pop('destination', ''), params)


def reset(request):
Expand Down

0 comments on commit 61a9bb5

Please sign in to comment.