fix: finish site editor cleanup (#810)

Author: wilhel1812

src/components/AccessSettingsEditor.test.tsx

@@ -111,4 +111,32 @@ describe("AccessSettingsEditor", () => {
     await userEvent.click(within(popover).getByRole("button", { name: /Add Bob/i }));
     expect(onAddCollaborator).toHaveBeenCalledWith("user-2");
   });
+
+  it("can disable collaborator removal without disabling role edits", async () => {
+    const onRemoveCollaborator = vi.fn();
+    const onRoleChange = vi.fn();
+    render(
+      <AccessSettingsEditor
+        canRemoveCollaborators={false}
+        collaborators={collaborators}
+        directory={directory}
+        onAddCollaborator={vi.fn()}
+        onRemoveCollaborator={onRemoveCollaborator}
+        onRoleChange={onRoleChange}
+        onVisibilityChange={vi.fn()}
+        visibility="private"
+      />,
+    );
+
+    await userEvent.click(screen.getByRole("button", { name: "Edit collaborators" }));
+    const popover = await screen.findByRole("dialog", { name: "Edit collaborators" });
+
+    await userEvent.selectOptions(within(popover).getByLabelText("Role for Alice"), "editor");
+    expect(onRoleChange).toHaveBeenCalledWith("user-1", "editor");
+
+    const remove = within(popover).getByRole("button", { name: "Remove Alice" });
+    expect(remove).toBeDisabled();
+    await userEvent.click(remove);
+    expect(onRemoveCollaborator).not.toHaveBeenCalled();
+  });
 });

src/components/AccessSettingsEditor.tsx

@@ -24,6 +24,7 @@ type AccessSettingsEditorProps = {
   onAddCollaborator: (userId: string) => void;
   onRemoveCollaborator: (userId: string) => void;
   onRoleChange: (userId: string, role: AccessRole) => void;
+  canRemoveCollaborators?: boolean;
   disabled?: boolean;
   ownerUserId?: string;
   directoryBusy?: boolean;
@@ -44,6 +45,7 @@ export function AccessSettingsEditor({
   onAddCollaborator,
   onRemoveCollaborator,
   onRoleChange,
+  canRemoveCollaborators = true,
   disabled = false,
   ownerUserId = "",
   directoryBusy = false,
@@ -193,7 +195,7 @@ export function AccessSettingsEditor({
                     <ActionButton
                       aria-label={`Remove ${label}`}
                       className="access-collaborator-remove"
-                      disabled={disabled}
+                      disabled={disabled || !canRemoveCollaborators}
                       onClick={() => onRemoveCollaborator(user.id)}
                       size="icon"
                       title={`Remove ${label}`}

src/components/MapView.tsx

@@ -806,6 +806,7 @@ export function MapView({
     bearing: number;
     pitch: number;
   } | null>(null);
+  const editorDraftAnimationKeyRef = useRef("");
 
   const stopUserLocation = useCallback(() => {
     if (userLocationWatchIdRef.current !== null && navigator.geolocation) {
@@ -2094,6 +2095,34 @@ export function MapView({
     latitude: viewport.center.lat,
     zoom: viewport.zoom,
   };
+  useEffect(() => {
+    if (!isMapLoaded || mapEditor?.kind !== "site" || !mapEditorSiteDraft) return;
+    const animationKey = `${mapEditor.resourceId ?? "new"}:${mapEditorSiteDraft.lat.toFixed(6)}:${mapEditorSiteDraft.lon.toFixed(6)}`;
+    if (editorDraftAnimationKeyRef.current === animationKey) return;
+    editorDraftAnimationKeyRef.current = animationKey;
+    setInteractionViewState(null);
+    const didAnimate = animateMapToCenter(mapRef, {
+      center: { lat: mapEditorSiteDraft.lat, lon: mapEditorSiteDraft.lon },
+      zoom: viewport.zoom,
+      padding: resolveMapCameraPadding(fitChromePadding, fitBottomInset),
+      duration: 420,
+    });
+    if (!didAnimate) {
+      updateMapViewport({
+        center: { lat: mapEditorSiteDraft.lat, lon: mapEditorSiteDraft.lon },
+        zoom: viewport.zoom,
+      });
+    }
+  }, [
+    fitBottomInset,
+    fitChromePadding,
+    isMapLoaded,
+    mapEditor?.kind,
+    mapEditor?.resourceId,
+    mapEditorSiteDraft,
+    updateMapViewport,
+    viewport.zoom,
+  ]);
   const mqttNodesInView = useMemo(() => {
     const lonSpan = Math.max(0.12, 360 / Math.pow(2, activeViewState.zoom) * 2.2);
     const latSpan = Math.max(0.12, 170 / Math.pow(2, activeViewState.zoom) * 1.8);
@@ -3424,9 +3453,17 @@ export function MapView({
         </Source>
 
         {sites.map((site) => {
-          const isSelected = !armAddSiteOnNextEmptyMapClick && selectedSiteSet.has(site.id);
+          const isEditedSiteInSimulation =
+            mapEditor?.kind === "site" &&
+            !mapEditor.isNew &&
+            mapEditor.resourceId !== null &&
+            site.libraryEntryId === mapEditor.resourceId &&
+            mapEditorSiteDraft !== null;
+          const isSelected = isEditedSiteInSimulation || (!armAddSiteOnNextEmptyMapClick && selectedSiteSet.has(site.id));
           const pendingMove = pendingSiteMoves[site.id];
-          const markerPosition = pendingMove?.currentPosition ?? site.position;
+          const markerPosition = isEditedSiteInSimulation
+            ? { lat: mapEditorSiteDraft.lat, lon: mapEditorSiteDraft.lon }
+            : pendingMove?.currentPosition ?? site.position;
           const isTemporarilyMoved = Boolean(pendingMove);
           const isPassFailMode = coverageVizMode === "passfail" && Boolean(selectedFromSite);
           const isRelayMode = coverageVizMode === "relay" && Boolean(selectedFromSite) && Boolean(selectedToSite);
@@ -3445,8 +3482,8 @@ export function MapView({
               longitude={markerPosition.lon}
               offset={SITE_PIN_MARKER_OFFSET}
               style={{ zIndex: markerZIndex }}
-              onDrag={(event) => onSiteDrag(site.id, event)}
-              onDragEnd={(event) => onSiteDragEnd(site.id, event)}
+              onDrag={isEditedSiteInSimulation ? undefined : (event) => onSiteDrag(site.id, event)}
+              onDragEnd={isEditedSiteInSimulation ? onEditorSiteDraftDragEnd : (event) => onSiteDragEnd(site.id, event)}
             >
               <MarkerActionButton
                 ariaLabel={site.name}
@@ -3556,7 +3593,9 @@ export function MapView({
           </Marker>
         ) : null}
 
-        {mapEditor?.kind === "site" && mapEditorSiteDraft ? (
+        {mapEditor?.kind === "site" &&
+        mapEditorSiteDraft &&
+        (mapEditor.isNew || !sites.some((site) => site.libraryEntryId === mapEditor.resourceId)) ? (
           <Marker
             anchor="bottom"
             draggable={canPersist}

src/components/MapView.userLocation.test.tsx

@@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 
 const mapMock = vi.hoisted(() => ({
   easeTo: vi.fn(),
+  markerProps: [] as Array<{ latitude?: number; longitude?: number }>,
   latestProps: null as null | {
     onMove?: (event: { originalEvent?: unknown; viewState: { longitude: number; latitude: number; zoom: number } }) => void;
   },
@@ -47,7 +48,18 @@ vi.mock("react-map-gl/maplibre", async () => {
       return <div data-testid="mock-map">{props.children}</div>;
     }),
     Layer: () => null,
-    Marker: ({ children }: { children?: React.ReactNode }) => <div>{children}</div>,
+    Marker: ({
+      children,
+      latitude,
+      longitude,
+    }: {
+      children?: React.ReactNode;
+      latitude?: number;
+      longitude?: number;
+    }) => {
+      mapMock.markerProps.push({ latitude, longitude });
+      return <div>{children}</div>;
+    },
     Source: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
   };
 });
@@ -98,6 +110,7 @@ describe("MapView user location flow", () => {
   beforeEach(() => {
     vi.clearAllMocks();
     mapMock.latestProps = null;
+    mapMock.markerProps = [];
     installGeolocation();
     watchPosition.mockReturnValue(42);
     Object.defineProperty(HTMLCanvasElement.prototype, "getContext", {
@@ -274,6 +287,42 @@ describe("MapView user location flow", () => {
     ).toBeInTheDocument();
   });
 
+  it("uses the existing simulation marker as the editor marker when editing its library site", () => {
+    useAppStore.setState({
+      sites: [
+        {
+          id: "site-alpha",
+          name: "Alpha Site",
+          libraryEntryId: "lib-alpha",
+          position: { lat: 60.5, lon: 11.5 },
+          groundElevationM: 120,
+          antennaHeightM: 2,
+          txPowerDbm: 20,
+          txGainDbi: 2,
+          rxGainDbi: 2,
+          cableLossDb: 1,
+        },
+      ],
+      selectedSiteId: "site-alpha",
+      selectedSiteIds: ["site-alpha"],
+      mapEditor: {
+        kind: "site",
+        resourceId: "lib-alpha",
+        isNew: false,
+        label: "Alpha Site",
+        anchorRect: { top: 0, right: 0, bottom: 0, left: 0, width: 0, height: 0 },
+      },
+      mapEditorSiteDraft: { lat: 61.25, lon: 12.75, groundElevationM: 130 },
+    });
+
+    renderMapView();
+
+    expect(screen.getAllByRole("button", { name: "Alpha Site" })).toHaveLength(1);
+    expect(mapMock.markerProps).toEqual(
+      expect.arrayContaining([expect.objectContaining({ latitude: 61.25, longitude: 12.75 })]),
+    );
+  });
+
   it("publishes plain location failure notifications", () => {
     const onPublishNotice = vi.fn();
     const consoleError = vi.spyOn(console, "error").mockImplementation(() => undefined);