Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
69 changes: 36 additions & 33 deletions apps/geolibre-desktop/src/components/layout/CollaborateDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
});
};

const [requireIdentity, setRequireIdentity] = useState(false);

const handleStart = async () => {
if (!name.trim()) {
setError(t("collaborate.nameRequired"));
Expand All @@ -122,10 +124,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
setBusy(true);
setError(null);
try {
await api.start(name.trim(), color, mode);
await api.start(name.trim(), color, mode, requireIdentity);
} catch (err) {
// Show a localized message; keep the raw error in the console for
// diagnostics (collab-client throws human-readable English strings).
console.error("[GeoLibre] Collaboration error", err);
setError(t("collaborate.connectFailed"));
} finally {
Expand All @@ -147,12 +147,8 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
try {
await api.join(code.trim(), name.trim(), color);
} catch (err) {
// Show a localized message; keep the raw error in the console for
// diagnostics (collab-client throws human-readable English strings).
console.error("[GeoLibre] Collaboration error", err);
setError(t("collaborate.connectFailed"));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor UX note: this PR adds several new join-rejection reasons the relay can send back (identity-required, forbidden for a blocked participant key), each with a specific message string (e.g. "You have been blocked from this session by the host.", "Sign-in required to join this session."). api.join now rejects with new Error(message.message) carrying that specific text (see the new pendingConnectRef handling in useCollaboration.ts), but this catch block discards it in favor of the generic t("collaborate.connectFailed"). A user who gets blocked or hits an identity gate sees the same "connection failed" message as a plain wrong invite code, with no way to tell why.

This pattern predates this PR (the removed comment even called it out as intentional), so it may be out of scope — but the set of distinguishable failure reasons has grown meaningfully with this feature, which makes surfacing err.message (or at least distinguishing "blocked"/"identity-required" specifically) more valuable than it was before.

Confidence: low-medium.

// The invite link could not connect (e.g. an expired or invalid code), so
// reveal the full layout and let the user fix the code or host instead.
setInvited(false);
} finally {
setBusy(false);
Expand Down Expand Up @@ -184,13 +180,13 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
onDismiss={() => onOpenChange(false)}
onSetMode={api.setMode}
onSetParticipantMode={api.setParticipantMode}
onKickParticipant={api.kickParticipant}
onBlockParticipant={api.blockParticipant}
onSetSessionConfig={api.setSessionConfig}
onSetFollowHost={api.setFollowHost}
/>
) : (
<div className="space-y-4">
{/* Name and color feed both actions below, so group them in a
shaded panel above the cards to read as shared profile inputs
rather than belonging to either Start or Join (#706). */}
<div className="space-y-3 rounded-md border bg-muted/40 p-3">
<div className="space-y-1.5">
<Label htmlFor="collab-name">{t("collaborate.displayName")}</Label>
Expand All @@ -206,8 +202,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</div>
<div className="space-y-1.5">
<Label>{t("collaborate.color")}</Label>
{/* Full panel width keeps every swatch on one row instead of
wrapping a lone dot to a second line (#706). */}
<div className="flex flex-wrap gap-2 pt-1">
{COLOR_PALETTE.map((c) => (
<button
Expand All @@ -216,9 +210,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
aria-label={c.name}
aria-pressed={color === c.hex}
onClick={() => setColor(c.hex)}
// Selection is an outer ring (offset from the swatch), so
// the colored circle stays the same size — a border would
// inset the fill and make the selected one look smaller.
className={`h-6 w-6 rounded-full transition ${
color === c.hex
? "ring-2 ring-offset-2 ring-offset-background ring-foreground"
Expand All @@ -231,10 +222,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</div>
</div>

{/* An invited participant (arrived via a `?collab=` link) only needs
to join, so collapse the layout to a single Join action and hide
the "Start a session" controls that are irrelevant to them
(#753). They can still fall back to hosting via the link below. */}
{invited ? (
<div className="space-y-3 rounded-md border p-3">
<div className="space-y-1">
Expand Down Expand Up @@ -269,9 +256,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</Button>
<button
type="button"
// Keep the prefilled `code` so the join field stays populated
// if the user changes their mind; the full layout shows Start
// as the primary action, so leaving it is non-destructive.
onClick={() => setInvited(false)}
disabled={busy}
className="cursor-pointer text-xs text-muted-foreground underline-offset-2 hover:underline disabled:cursor-default disabled:opacity-50"
Expand All @@ -295,6 +279,16 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
<option value="view-only">{t("collaborate.modeViewOnly")}</option>
</Select>
</div>
<label className="flex cursor-pointer items-center gap-2 text-xs text-muted-foreground pt-1">
<input
type="checkbox"
checked={requireIdentity}
onChange={(e) => setRequireIdentity(e.target.checked)}
disabled={busy}
className="h-3.5 w-3.5 rounded border"
/>
Require signed-in account to join
Comment thread
HarshShinde0 marked this conversation as resolved.
Outdated
</label>
Comment thread
HarshShinde0 marked this conversation as resolved.
Outdated
<Button
type="button"
onClick={() => void handleStart()}
Expand Down Expand Up @@ -333,9 +327,6 @@ export function CollaborateDialog({ open, onOpenChange, api }: CollaborateDialog
</>
)}

{/* Show local validation errors and connect failures, the latter
arriving asynchronously in the store (the WebSocket handshake
fails after this dialog's call already resolved). */}
{(error || collaboration.error) && (
<p className="rounded-md bg-destructive/10 p-2 text-sm text-destructive">
{error || collaboration.error}
Expand All @@ -356,6 +347,9 @@ function ActiveSession({
onDismiss,
onSetMode,
onSetParticipantMode,
onKickParticipant,
onBlockParticipant,
onSetSessionConfig,
onSetFollowHost,
}: {
shareLink: string;
Expand All @@ -365,6 +359,9 @@ function ActiveSession({
onDismiss: () => void;
onSetMode: (mode: CollaborationMode) => void;
onSetParticipantMode: (clientId: string, canEdit: boolean) => void;
onKickParticipant?: (clientId: string) => void;
onBlockParticipant?: (clientId: string) => void;
onSetSessionConfig?: (config: { requireIdentity?: boolean }) => void;
onSetFollowHost: (enabled: boolean) => void;
}) {
const { t } = useTranslation();
Expand All @@ -387,8 +384,6 @@ function ActiveSession({
)}
</div>

{/* Cameras are independent by default; a non-host can opt to follow the
host's viewport (presenter mode). */}
{!isHost && (
<label className="flex cursor-pointer items-center gap-2 text-sm">
<input
Expand All @@ -401,6 +396,18 @@ function ActiveSession({
</label>
)}

{isHost && (
<label className="flex cursor-pointer items-center gap-2 text-xs text-muted-foreground">
<input
type="checkbox"
checked={collaboration.requireIdentity}
onChange={(e) => onSetSessionConfig?.({ requireIdentity: e.target.checked })}
className="h-3.5 w-3.5 rounded border"
/>
Require signed-in account to join
</label>
)}

<div className="space-y-1.5">
<Label>{t("collaborate.sessionCode")}</Label>
<div className="flex gap-2">
Expand Down Expand Up @@ -441,7 +448,6 @@ function ActiveSession({
)}
</Button>
</div>
{/* Only the host invites others, so the scan-to-join QR is host-only. */}
{isHost && (
<div className="flex flex-col items-center gap-1.5 pt-1">
<div className="rounded-md bg-white p-2">
Expand Down Expand Up @@ -472,6 +478,8 @@ function ActiveSession({
isSelf={p.clientId === collaboration.clientId}
canManage={isHost}
onSetParticipantMode={onSetParticipantMode}
onKickParticipant={onKickParticipant}
onBlockParticipant={onBlockParticipant}
/>
))}
</ul>
Expand All @@ -483,12 +491,7 @@ function ActiveSession({
</p>
)}

{/* Primary way out of the dialog: dismiss it while keeping the session
live, so the host isn't tempted to use the "X" (which they fear ends
the session) to get back to the map (#754). */}
<Button type="button" className="w-full" onClick={onDismiss}>
{/* A view-only guest cannot edit, so "collaborate" would mislead; offer
"watch" wording for that case. */}
{isHost || collaboration.mode === "co-edit"
? t("collaborate.goToMap")
: t("collaborate.goToMapViewOnly")}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { CollaborationMode, CollaborationParticipant } from "@geolibre/core";
import { Eye, Pencil } from "lucide-react";
import { Eye, Pencil, UserX, Ban } from "lucide-react";
import { useTranslation } from "react-i18next";
import { participantCanEdit } from "../../lib/collab-protocol";

Expand All @@ -14,6 +14,8 @@ interface CollaborationParticipantRowProps {
* own row never shows a control. */
canManage: boolean;
onSetParticipantMode: (clientId: string, canEdit: boolean) => void;
onKickParticipant?: (clientId: string) => void;
onBlockParticipant?: (clientId: string) => void;
/** Render the smaller variant used in the on-canvas badge roster. */
compact?: boolean;
}
Expand All @@ -30,6 +32,8 @@ export function CollaborationParticipantRow({
isSelf,
canManage,
onSetParticipantMode,
onKickParticipant,
onBlockParticipant,
compact = false,
}: CollaborationParticipantRowProps) {
const { t } = useTranslation();
Expand All @@ -51,6 +55,11 @@ export function CollaborationParticipantRow({
style={{ backgroundColor: p.color }}
/>
<span className="truncate">{p.displayName}</span>
{p.identity && (
<span className="rounded bg-primary/10 px-1 text-[10px] text-primary">
{p.identity.provider === "geolibre" ? "✓" : p.identity.provider}
</span>
)}
{isSelf && <span className="text-xs text-muted-foreground">({t("collaborate.you")})</span>}
{isHostRow && (
<span
Expand All @@ -61,19 +70,43 @@ export function CollaborationParticipantRow({
)}
{!isHostRow &&
(showToggle ? (
<button
type="button"
onClick={() => onSetParticipantMode(p.clientId, !editable)}
// A binary "can edit" vs "view-only" setting reads as a switch to
// assistive tech, rather than a momentary press (aria-pressed).
role="switch"
aria-checked={editable}
title={editable ? t("collaborate.setViewOnly") : t("collaborate.allowEdit")}
className={`ms-auto flex shrink-0 items-center gap-1 rounded border py-0.5 text-muted-foreground transition hover:bg-accent hover:text-foreground ${compact ? "px-1 text-[10px]" : "px-1.5 text-xs"}`}
>
{permIcon}
{permLabel}
</button>
<div className="ms-auto flex shrink-0 items-center gap-1">
<button
type="button"
onClick={() => onSetParticipantMode(p.clientId, !editable)}
role="switch"
aria-checked={editable}
title={editable ? t("collaborate.setViewOnly") : t("collaborate.allowEdit")}
className={`flex items-center gap-1 rounded border py-0.5 text-muted-foreground transition hover:bg-accent hover:text-foreground ${compact ? "px-1 text-[10px]" : "px-1.5 text-xs"}`}
>
{permIcon}
{permLabel}
</button>
{canManage && !compact && (
<>
{onKickParticipant && (
<button
type="button"
onClick={() => onKickParticipant(p.clientId)}
title={(t as (key: string) => string)("collaborate.kick")}
className="rounded p-1 text-muted-foreground hover:bg-destructive/10 hover:text-destructive"
>
<UserX className="h-3 w-3" />
</button>
)}
{onBlockParticipant && (
<button
type="button"
onClick={() => onBlockParticipant(p.clientId)}
title={(t as (key: string) => string)("collaborate.block")}
className="rounded p-1 text-muted-foreground hover:bg-destructive/10 hover:text-destructive"
>
<Ban className="h-3 w-3" />
</button>
Comment thread
HarshShinde0 marked this conversation as resolved.
)}
</>
)}
</div>
) : (
// Non-host viewers still see each guest's current permission.
<span
Expand Down
Loading
Loading