Hide write-action controls in the tree UI when logged out
Delete (file and folder), download, upload, and new-folder were all rendered unconditionally and relied on the backend's 401 + a redirect to /login on click -- fine for surfacing the reason something failed, but showing controls that plainly won't do anything until you log in was the actual complaint. isLoggedIn() checks for the topbar's own "Log out" link (only rendered when logged_in) instead of threading a new flag through -- already there, and login state can't change without a full page reload anyway. Caught a real gap while verifying in a real browser: the folder row's own delete button wasn't gated (only the file row's was), so a "logged out" check still showed a delete icon on every folder. Fixed before it shipped -- confirmed clean in both states afterward.
This commit is contained in:
1 parent
058c0990b4
commit
bf17e00463
2 files changed
+43
-10
No files matched your search
@@ -691,6 +691,15 @@ require login, guarded by `require_login!` in `app.rb` — and so does
|
|||||||
exporting a file (`GET /api/gpx/file/*?download=...`), even though
|
exporting a file (`GET /api/gpx/file/*?download=...`), even though
|
||||||
plain viewing of that same URL (no `?download`) doesn't; not a write
|
plain viewing of that same URL (no `?download`) doesn't; not a write
|
||||||
exactly, but treated the same as the others rather than left open.
|
exactly, but treated the same as the others rather than left open.
|
||||||
|
The tree UI (`public/js/gpx.js`) mirrors this: every control for a
|
||||||
|
gated action (delete, download, upload, new folder) is left out of
|
||||||
|
the DOM entirely when logged out rather than shown and failing on
|
||||||
|
click — `isLoggedIn()` checks for the topbar's own "Log out" link
|
||||||
|
(`views/index.erb` only renders one when `logged_in`) instead of
|
||||||
|
threading a separate flag through, since that's already there and
|
||||||
|
login state can't change without a full page reload anyway. Delete
|
||||||
|
exists on both file and folder rows — both are gated, easy to miss
|
||||||
|
one and only catch it by testing an actually-logged-out session.
|
||||||
|
|
||||||
Login delegates to the Gitea instance at `git.priesol.net` via OAuth2
|
Login delegates to the Gitea instance at `git.priesol.net` via OAuth2
|
||||||
(`GET /login` redirects to Gitea's authorize endpoint, `GET
|
(`GET /login` redirects to Gitea's authorize endpoint, `GET
|
||||||
|
|||||||
+34
-10
@@ -31,6 +31,17 @@ const BADGE_IMAGE_PREFIX = "badge-"; // one registered image per distinct badge
|
|||||||
// one — both land on the same drop targets, told apart by this.
|
// one — both land on the same drop targets, told apart by this.
|
||||||
const MOVE_MIME = "application/x-gpx-path";
|
const MOVE_MIME = "application/x-gpx-path";
|
||||||
|
|
||||||
|
// No server-rendered flag for this exists yet, and adding one would be more
|
||||||
|
// plumbing than reusing what's already on the page: the topbar's own
|
||||||
|
// logged_in check (views/index.erb) already renders a "Log out" link only
|
||||||
|
// when logged in. Write-action controls (delete/download/upload/new
|
||||||
|
// folder) check this to hide themselves entirely rather than being shown
|
||||||
|
// and failing on click — login state never changes without a full page
|
||||||
|
// reload, so this only needs checking once per render, not reactively.
|
||||||
|
function isLoggedIn() {
|
||||||
|
return !!document.querySelector('a[href="/logout"]');
|
||||||
|
}
|
||||||
|
|
||||||
// A sidebar icon that opens a side panel for uploading GPX files (via
|
// A sidebar icon that opens a side panel for uploading GPX files (via
|
||||||
// drag & drop, stored server-side in a folder hierarchy the user can add to
|
// drag & drop, stored server-side in a folder hierarchy the user can add to
|
||||||
// and reorganize by dragging), and toggling their tracks/waypoints on and
|
// and reorganize by dragging), and toggling their tracks/waypoints on and
|
||||||
@@ -109,6 +120,10 @@ export class GpxPanelControl {
|
|||||||
this._uploadStatus = this._panel.querySelector(".gpx-upload__status");
|
this._uploadStatus = this._panel.querySelector(".gpx-upload__status");
|
||||||
this._panel.querySelector('[data-action="new-folder"]').innerHTML = NEW_FOLDER_ICON;
|
this._panel.querySelector('[data-action="new-folder"]').innerHTML = NEW_FOLDER_ICON;
|
||||||
this._panel.querySelector('[data-action="upload"]').innerHTML = UPLOAD_ICON;
|
this._panel.querySelector('[data-action="upload"]').innerHTML = UPLOAD_ICON;
|
||||||
|
if (!isLoggedIn()) {
|
||||||
|
this._panel.querySelector('[data-action="new-folder"]').hidden = true;
|
||||||
|
this._panel.querySelector('[data-action="upload"]').hidden = true;
|
||||||
|
}
|
||||||
this._panel
|
this._panel
|
||||||
.querySelector(".gpx-panel__close")
|
.querySelector(".gpx-panel__close")
|
||||||
.addEventListener("click", () => this._togglePanel(false));
|
.addEventListener("click", () => this._togglePanel(false));
|
||||||
@@ -249,7 +264,7 @@ export class GpxPanelControl {
|
|||||||
const name = document.createElement("span");
|
const name = document.createElement("span");
|
||||||
name.textContent = folder.name;
|
name.textContent = folder.name;
|
||||||
row.appendChild(name);
|
row.appendChild(name);
|
||||||
row.appendChild(this._deleteButton(() => this._deleteFolder(folder.path)));
|
if (isLoggedIn()) row.appendChild(this._deleteButton(() => this._deleteFolder(folder.path)));
|
||||||
summary.appendChild(row);
|
summary.appendChild(row);
|
||||||
this._wireFolderDropTarget(summary, folder.path);
|
this._wireFolderDropTarget(summary, folder.path);
|
||||||
|
|
||||||
@@ -277,34 +292,43 @@ export class GpxPanelControl {
|
|||||||
// name reflects that (matching the real "<path>.geojson" cache file on
|
// name reflects that (matching the real "<path>.geojson" cache file on
|
||||||
// disk) rather than still reading as a plain, un-converted .gpx/.fit.
|
// disk) rather than still reading as a plain, un-converted .gpx/.fit.
|
||||||
// A direct .geojson upload already ends in .geojson, so it's left
|
// A direct .geojson upload already ends in .geojson, so it's left
|
||||||
// alone. It's an <a>, not a <span>: clicking it downloads that GeoJSON
|
// alone.
|
||||||
// — the only way to get it out of the app, since the original .gpx/.fit
|
const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
|
||||||
|
const loggedIn = isLoggedIn();
|
||||||
|
// Logged in: an <a>, not a <span> — clicking it downloads that GeoJSON,
|
||||||
|
// the only way to get it out of the app, since the original .gpx/.fit
|
||||||
// is never deleted but also never re-shown once converted. An <a>
|
// is never deleted but also never re-shown once converted. An <a>
|
||||||
// nested in a <label> doesn't also toggle the checkbox on click (unlike
|
// nested in a <label> doesn't also toggle the checkbox on click (unlike
|
||||||
// a plain element), so this doesn't fight the row's own
|
// a plain element), so this doesn't fight the row's own
|
||||||
// toggle-on-map behavior. href kept pointing at the real endpoint (so
|
// toggle-on-map behavior. href kept pointing at the real endpoint (so
|
||||||
// hover/right-click/middle-click still do something sensible) but the
|
// hover/right-click/middle-click still do something sensible) but the
|
||||||
// actual click goes through _download, which — like every other write
|
// actual click goes through _download, which redirects to /login on a
|
||||||
// action in this file — needs to redirect to /login on a 401 rather
|
// 401 rather than navigating to a raw JSON error page (this endpoint
|
||||||
// than navigate to a raw JSON error page (this endpoint now requires
|
// requires login the same as upload/move/delete, see app.rb).
|
||||||
// login the same as upload/move/delete, see app.rb).
|
// Logged out: plain text — nothing here would actually work, so it
|
||||||
const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
|
// isn't shown as if it would (same reasoning as the buttons below).
|
||||||
const name = document.createElement("a");
|
const name = document.createElement(loggedIn ? "a" : "span");
|
||||||
name.className = "gpx-tree__file-name";
|
name.className = "gpx-tree__file-name";
|
||||||
name.href = `${gpxFileUrl(file.path)}?download=geojson`;
|
|
||||||
name.textContent = displayName;
|
name.textContent = displayName;
|
||||||
|
if (loggedIn) {
|
||||||
|
name.href = `${gpxFileUrl(file.path)}?download=geojson`;
|
||||||
name.title = `Download ${displayName}`;
|
name.title = `Download ${displayName}`;
|
||||||
name.addEventListener("click", (event) => {
|
name.addEventListener("click", (event) => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
this._download(name.href, displayName);
|
this._download(name.href, displayName);
|
||||||
});
|
});
|
||||||
|
}
|
||||||
label.appendChild(checkbox);
|
label.appendChild(checkbox);
|
||||||
(file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind)));
|
(file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind)));
|
||||||
if (!file.cached) label.appendChild(this._pendingIcon());
|
if (!file.cached) label.appendChild(this._pendingIcon());
|
||||||
label.appendChild(name);
|
label.appendChild(name);
|
||||||
row.appendChild(label);
|
row.appendChild(label);
|
||||||
|
// Delete/download require login (app.rb); shown only once logged in
|
||||||
|
// rather than shown-but-failing-on-click.
|
||||||
|
if (loggedIn) {
|
||||||
row.appendChild(this._downloadButton(file));
|
row.appendChild(this._downloadButton(file));
|
||||||
row.appendChild(this._deleteButton(() => this._deleteFile(file.path)));
|
row.appendChild(this._deleteButton(() => this._deleteFile(file.path)));
|
||||||
|
}
|
||||||
|
|
||||||
return row;
|
return row;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user