Merge pull request #7298 from hotosm/develop

iD editor related updates to staging server

Author: ramyaragupathy

frontend/src/components/editor.js

@@ -1,4 +1,4 @@
-import { useEffect, useLayoutEffect, useState } from 'react';
+import { useEffect, useState } from 'react';
 import { useSelector, useDispatch } from 'react-redux';
 import { useIntl } from 'react-intl';
 import { gpx } from '@tmcw/togeojson';
@@ -7,19 +7,9 @@ import '@openstreetmap/id/dist/iD.css';
 
 import { OSM_CLIENT_ID, OSM_REDIRECT_URI, OSM_SERVER_URL } from '../config';
 import messages from './messages';
-import {
-  registerIdEditorStylesheet,
-  activateIdEditorStylesheet,
-} from '../utils/idEditorStylesheets';
+import { captureIdEditorPackage, resolveIdEditorContext } from '../utils/idEditorContext';
 
-// @openstreetmap/id has no real ESM/CJS exports — it only assigns itself to
-// window.iD as a side effect on import. Capture it here, right after the
-// import above forces that assignment, so this module keeps its own private
-// reference instead of the shared global, which @osm-sandbox/sandbox-id
-// (imported by sandboxEditor.js) later overwrites.
-const officialID = window.iD;
-
-registerIdEditorStylesheet('official');
+const officialID = captureIdEditorPackage();
 
 export default function Editor({ setDisable, comment, presets, imagery, gpxUrl, extraIdParams }) {
   const dispatch = useDispatch();
@@ -32,14 +22,6 @@ export default function Editor({ setDisable, comment, presets, imagery, gpxUrl,
   const customSource =
     iDContext && iDContext.background() && iDContext.background().findSource('custom');
 
-  // Only one of the OSM iD editor / Sandbox editor is ever mounted at a
-  // time, but both of their stylesheets stay loaded for the whole page
-  // session once visited. Disable the other one's so its rules can't leak
-  // into this editor via their shared ".ideditor" root class.
-  useLayoutEffect(() => {
-    activateIdEditorStylesheet('official');
-  }, []);
-
   useEffect(() => {
     if (!customImageryIsSet && imagery && customSource) {
       if (imagery.startsWith('http')) {
@@ -83,31 +65,21 @@ export default function Editor({ setDisable, comment, presets, imagery, gpxUrl,
   }, [customImageryIsSet, imagery, iDContext, customSource, extraIdParams]);
 
   useEffect(() => {
-    if (windowInit) {
-      if (iDContext === null) {
-        // we need to keep iD context on redux store because iD works better if
-        // the context is not restarted while running in the same browser session
-        dispatch({ type: 'SET_EDITOR', context: officialID.coreContext() });
-      }
+    if (!windowInit) return;
+    const context = resolveIdEditorContext(iDContext, 'official', () => officialID.coreContext());
+    if (context && context !== iDContext) {
+      dispatch({ type: 'SET_EDITOR', context });
     }
   }, [windowInit, iDContext, dispatch]);
 
-  // Reset context on unmount so the sandbox editor always gets a fresh context
-  // from its own iD module (sandbox-id), preventing cross-editor context bleed.
-  useEffect(() => {
-    return () => {
-      dispatch({ type: 'SET_EDITOR', context: null });
-    };
-  }, [dispatch]);
-
   useEffect(() => {
     if (iDContext && comment) {
       iDContext.defaultChangesetComment(comment);
     }
   }, [comment, iDContext]);
 
   useEffect(() => {
-    if (session && locale && iD && iDContext) {
+    if (session && locale && iD && iDContext && iDContext.__idEditorType === 'official') {
       // if presets is not a populated list we need to set it as null
       try {
         if (presets.length) {
@@ -125,7 +97,10 @@ export default function Editor({ setDisable, comment, presets, imagery, gpxUrl,
         .locale(locale)
         .setsDocumentTitle(false)
         .containerNode(document.getElementById('id-container'));
-      // init the ui or restart if it was loaded previously
+      // init the ui or restart if it was loaded previously. Either path ends
+      // with osm.switch() below, which resets the (module-scoped, session-wide)
+      // tile cache and triggers a fresh load from the current view — so a
+      // reused/fresh context never serves stale, pre-edit data.
       if (iDContext.ui() !== undefined) {
         iDContext.reset();
         iDContext.ui().restart();

frontend/src/components/sandboxEditor.js

@@ -1,4 +1,4 @@
-import { useEffect, useLayoutEffect, useState } from 'react';
+import { useEffect, useState } from 'react';
 import { useNavigate } from 'react-router-dom';
 import { useSelector, useDispatch } from 'react-redux';
 import { useIntl } from 'react-intl';
@@ -15,19 +15,9 @@ import {
 import { useSandboxOAuthCallback } from '../hooks/UseSandboxOAuthCallback';
 import { getValidTokenOrInitiateAuth, fetchSandboxLicense } from '../utils/sandboxUtils';
 import { useOsmFeaturesQuery } from '../api/projects';
-import {
-  registerIdEditorStylesheet,
-  activateIdEditorStylesheet,
-} from '../utils/idEditorStylesheets';
-
-// @osm-sandbox/sandbox-id has no real ESM/CJS exports — it only assigns
-// itself to window.iD as a side effect on import. Capture it here, right
-// after the imports above force that assignment, so this module keeps its
-// own private reference instead of the shared global, which
-// @openstreetmap/id (imported by editor.js) later overwrites.
-const sandboxID = window.iD;
+import { captureIdEditorPackage, resolveIdEditorContext } from '../utils/idEditorContext';
 
-registerIdEditorStylesheet('sandbox');
+const sandboxID = captureIdEditorPackage();
 
 export default function SandboxEditor({
   setDisable,
@@ -62,14 +52,6 @@ export default function SandboxEditor({
 
   useSandboxOAuthCallback(sandboxId);
 
-  // Only one of the OSM iD editor / Sandbox editor is ever mounted at a
-  // time, but both of their stylesheets stay loaded for the whole page
-  // session once visited. Disable the other one's so its rules can't leak
-  // into this editor via their shared ".ideditor" root class.
-  useLayoutEffect(() => {
-    activateIdEditorStylesheet('sandbox');
-  }, []);
-
   const customSource =
     iDContext && iDContext.background() && iDContext.background().findSource('custom');
 
@@ -90,10 +72,9 @@ export default function SandboxEditor({
   }, [customImageryIsSet, imagery, iDContext, customSource]);
 
   useEffect(() => {
-    if (iDContext === null) {
-      // we need to keep iD context on redux store because iD works better if
-      // the context is not restarted while running in the same browser session
-      dispatch({ type: 'SET_EDITOR', context: sandboxID.coreContext() });
+    const context = resolveIdEditorContext(iDContext, 'sandbox', () => sandboxID.coreContext());
+    if (context && context !== iDContext) {
+      dispatch({ type: 'SET_EDITOR', context });
     }
   }, [iDContext, dispatch]);
 
@@ -106,7 +87,14 @@ export default function SandboxEditor({
   // Initialize sandbox editor
   useEffect(() => {
     const initializeSandbox = async () => {
-      if (!session || !locale || !iD || !iDContext || isInitialized) {
+      if (
+        !session ||
+        !locale ||
+        !iD ||
+        !iDContext ||
+        iDContext.__idEditorType !== 'sandbox' ||
+        isInitialized
+      ) {
         return;
       }
       const authStatus = sandboxAuthStatus?.[sandboxId];
@@ -151,7 +139,16 @@ export default function SandboxEditor({
           .setsDocumentTitle(false)
           .containerNode(document.getElementById('id-container'));
 
-        // init the ui or restart if it was loaded previously
+        // @osm-sandbox/sandbox-id has no dark theme of its own, but it shares the
+        // "ideditor" root class with @openstreetmap/id, which does have one keyed
+        // off the OS/browser's prefers-color-scheme. Force light explicitly so
+        // this editor doesn't pick up a dark theme it was never styled for.
+        iDContext.container().classed('theme-light', true);
+
+        // init the ui or restart if it was loaded previously. Either path ends
+        // with connection().switch() below, which resets the (module-scoped,
+        // session-wide) tile cache and triggers a fresh load from the current
+        // view — so a reused/fresh context never serves stale, pre-edit data.
         if (iDContext.ui() !== undefined) {
           iDContext.reset();
           iDContext.ui().restart();
@@ -288,9 +285,6 @@ export default function SandboxEditor({
     return () => {
       // Reset auth status for this sandbox on unmount
       dispatch(setSandboxAuthStatus(sandboxId, 'idle'));
-      // Reset context on unmount so the OSM iD editor always gets a fresh context
-      // from its own iD module (@openstreetmap/id), preventing cross-editor context bleed.
-      dispatch({ type: 'SET_EDITOR', context: null });
     };
   }, [dispatch, sandboxId]);
 

frontend/src/utils/idEditorContext.js

@@ -0,0 +1,40 @@
+// Both @openstreetmap/id and @osm-sandbox/sandbox-id have no real ESM/CJS
+// exports — each only assigns itself to window.iD as a side effect on
+// import, and whichever loaded last wins that global for the rest of the
+// session. Call this at module scope, immediately after importing a
+// package's own module, so it captures that package's own reference before
+// the other one can overwrite the global.
+export function captureIdEditorPackage() {
+  return window.iD;
+}
+
+// The OSM iD editor and the Sandbox editor keep their iD context on the redux
+// store rather than recreating it on every mount — iD works better warm, and
+// reopening the same task shouldn't need a page reload to see recently-saved
+// edits. But the two editors can't share a context: they're built by different
+// iD forks, so a context from the other editor is missing methods it never
+// calls (e.g. Sandbox's own `.license()`). Each context is tagged with who
+// built it.
+//
+// This resolves the context for a mounting editor:
+//  - same type as the stored context  → reuse it (stays warm, no reload)
+//  - no stored context yet (fresh page load) → build a new tagged one
+//  - a DIFFERENT type is stored (a cross-editor switch within a live SPA
+//    session) → force a full page reload and return null. Initializing a
+//    fresh fork into the container mid-SPA-transition is unreliable (the
+//    task's own features sometimes never load until a manual reload), whereas
+//    a full reload lands back on the same task+editor via the URL and
+//    initializes cleanly — the one path that always works. On that reload the
+//    store is empty, so we take the "build a new one" branch, not this one.
+export function resolveIdEditorContext(existingContext, editorType, buildContext) {
+  if (existingContext) {
+    if (existingContext.__idEditorType === editorType) {
+      return existingContext;
+    }
+    window.location.reload();
+    return null;
+  }
+  const context = buildContext();
+  context.__idEditorType = editorType;
+  return context;
+}