fix: 1929 - follow on fixes to pr 1922 by ifsimicoded · Pull Request #1950 · NASA-IMPACT/veda-ui

ifsimicoded · 2025-12-08T22:02:55Z

Related Ticket:
Relates to the work completed in PR #1922
Closes #1929

Description of Changes

This PR addresses some of the issues that came up when the "embed" mode was created for the GHGc portal. Details in the related ticket. Note, because the entire app is 'embeddable', I've changed the name of this view to be 'simple' mode. (Most of the changes are renames).

⚠️ @slesaad Note that I've changed the URL parameter to viewMode=simple, so this will need to be updated in downstream consumers when the package is updated.

Validation / Testing

Ensure viewMode=simple renders the simple view, and any other viewMode renders the usual E&A view. embed should no longer alter the view.

netlify · 2025-12-08T22:03:01Z

✅ Deploy Preview for veda-ui ready!

Name	Link
🔨 Latest commit	`bb2186f`
🔍 Latest deploy log	https://app.netlify.com/projects/veda-ui/deploys/693871f46f1645000876df86
😎 Deploy Preview	https://deploy-preview-1950--veda-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

ifsimicoded · 2025-12-09T03:25:55Z

app/scripts/components/exploration/atoms/viewMode.ts

+const hydrateViewMode = (serialized: string | null) => {
+  if (serialized === 'simple') return serialized;
+  return 'default';
+};
+
+const dehydrateViewMode = (value: 'simple' | 'default') => {
+  return value ?? 'default';
+};
+
+export const viewModeAtom = atomWithUrlValueStability<'simple' | 'default'>({
+  initialValue: hydrateViewMode(initialParams.get('viewMode')),
+  urlParam: 'viewMode',
+  hydrate: hydrateViewMode,
+  dehydrate: dehydrateViewMode,
+  areEqual: (prev, next) => prev === next
+});


This file is largely the same as the embed atom, however, we're now managing a view mode (simple or default) vs a boolean. This allows for more view modes in the future

ifsimicoded · 2025-12-09T03:32:27Z

app/scripts/components/exploration/container.tsx

        title='Exploration'
        description='Explore and analyze datasets'
        hideFooter
+        {...(viewMode === 'simple' && { hideNav: true, hideHeader: true })}


This is the intended way to hide the header and footer, such that the simpleView can be exported independently, and not need to manage the header and footer / or interfere with SPA behavior.

ifsimicoded · 2025-12-09T03:33:26Z

app/scripts/components/exploration/views/simple/index.tsx

+              selectedCompareDay
+                ? 'Date shown on left map'
+                : 'Date shown on map'


fixes the tipContent issue

ifsimicoded · 2025-12-09T03:33:44Z

app/scripts/components/exploration/views/simple/index.tsx

            label=''
            date={selectedCompareDay}
            setDate={setSelectedComparedDay}
            datasets={layers as TimelineDataset[]}
            timeDensity={compareTimeDensity}
+            tipContent='Date shown on right map'


fixes tip content issue.

vgeorge

@ifsimicoded this is looking good. Some notes:

Considering ExplorarionAndAnalysis and ExplorationSimpleView now have the same hierarchy level, it might make sense to enhance the folder structure further. Example:

/exploration/
├── index.tsx                    # Container (stays at root)
├── views/                          # New: Views directory
│   ├── default/                      # Default E&A view
│   │   └── index.tsx                # ExplorationAndAnalysis
│   └── simple/                      # Simple view
│       ├── index.tsx
│       └── timeline-simple-view.tsx

I noticed that even when setting LayoutProps to hide layout components, there is still the banner and the site alert, which makes the simple view to slitghly offset and scroll. Not sure if we should address in this PR tough.

This PR will supersede #1931, could we use this opportunity to add a bit more JSDoc on the exported helpers/types (view mode utils, props, etc.)? The code is already very readable, but this would improve IntelliSense in editors and generate autodocs in the future.

vgeorge · 2025-12-09T14:46:04Z

...cripts/components/exploration/components/exploration-simple-view/exploration-simple-view.tsx

 };

-function EmbeddedLayersExploration(props: EmbeddedLayersExplorationProps) {
+function ExplorationSimpleView_(props: ExplorationSimpleViewProps_) {


I suggest to use a more descript name here, maybe ExplorationSimpleviewContent?

vgeorge · 2025-12-09T15:15:50Z

...cripts/components/exploration/components/exploration-simple-view/exploration-simple-view.tsx

 import { useVedaUI } from '$context/veda-ui-provider';

-const Carto = styled.div`
+const StyledDiv = styled.div`


I think we can use a more descriptive name as Container and remove StyledDiv from StyledDivTimelineContainer and StyledDivCompareTimelineContainer.

Can do! I do like to prefix Styled on components that are simply styles this way it's obvious this component contains to no functionality. It makes debugging simpler in component inspect view.

ifsimicoded · 2025-12-09T16:28:40Z

1. Considering ExplorarionAndAnalysis and ExplorationSimpleView now have the same hierarchy level, it might make sense to enhance the folder structure further. Example:

👍

I noticed that even when setting LayoutProps to hide layout components, there is still the banner and the site alert, which makes the simple view to slitghly offset and scroll. Not sure if we should address in this PR tough.

Will do in follow on PR.

This PR will supersede docs: add JSDoc comments to E&A embed mode implementation #1931, could we use this opportunity to add a bit more JSDoc on the exported helpers/types (view mode utils, props, etc.)? The code is already very readable, but this would improve IntelliSense in editors and generate autodocs in the future.

Will rely on you to complete this since you have a better idea of what you're looking for. Could you do that in a follow on?

ifsimicoded · 2025-12-09T16:31:18Z

app/scripts/components/exploration/views/default/index.tsx

@@ -0,0 +1,148 @@
+import React, { useEffect, useState } from 'react';


This file is largely the same as the prior exploration/index view but I updated all the relative path imports.

ifsimicoded · 2025-12-09T16:32:31Z

app/scripts/components/exploration/index.tsx

-    }
-  }
-`;
+import { DatasetSelectorModal } from '$components/exploration/components/dataset-selector-modal';


This file is largely the same as the prior container but I updated all the relative path imports.

Add JSDoc documentation to public API exports for the new view mode system, following existing patterns in the codebase. Documents: - viewModeAtom and ViewMode type - ExplorationAndAnalysisSimpleView and ExplorationAndAnalysisDefaultView - TimelineSimpleView component - Container routing logic Supersedes JSDoc work from #1931 after file restructuring.

vgeorge

@ifsimicoded thanks for the updates, I’ve added JSDocs to the public API exports to give more context on why they exist. I don’t think we need to add JSDocs for internal code if it’s already readable, which seems to be the case here.

One note: this may introduce a potential breaking change if embed=true is already being used downstream. We should confirm it won’t cause issues, or add a fallback to handle embed=true.

Aside from that, this looks good to merge.

ifsimicoded · 2025-12-09T18:15:30Z

@ifsimicoded thanks for the updates, I’ve added JSDocs to the public API exports to give more context on why they exist. I don’t think we need to add JSDocs for internal code if it’s already readable, which seems to be the case here.

🙏

One note: this may introduce a potential breaking change if embed=true is already being used downstream. We should confirm it won’t cause issues, or add a fallback to handle embed=true.

Yep, I noted that in the description:

But I'll reach out to Slesa

ifsimicoded · 2025-12-09T18:52:38Z

app/scripts/components/exploration/index.tsx

- * @NOTE: This container component serves as a wrapper for the purpose of data management, this is ONLY to support current instances.
- * veda2 instances can just use the direct component, 'ExplorationAndAnalysis', and manage data directly in their page views
+ * @NOTE: This container component serves as a wrapper for the purpose of data management,
+ * this is ONLY to support current instances. veda2 instances can just use the direct


@vgeorge what is current instances vs veda2? is current instances, veda as a submodule? and veda2 is the the next-veda-ui implementation as a library (instead of submodule)?

when you say current instances, if I read this in one year, will that still apply?

@ifsimicoded my original intent with this change was just to rename the reference component, and I was also confused by the veda2 term. After digging through past tickets, I realized it refers to a refactor effort from last year to evolve the architecture. Since this wasn’t well documented, I created a new ADR to capture what was done and how it connects to the current architecture discussions in #1952.

slesaad and others added 4 commits December 3, 2025 16:40

Update LayoutProps in container rather than in embedded-exploration

d5b1c8c

Fix datepicker tooltip content

3a6856f

Move embed-exploration to embed-exploration folder

2d87fdb

Merge branch 'main' into fix/1929/follow_on_fixes_to_PR1922

dbe622d

ifsimicoded added 3 commits December 8, 2025 14:47

rename embed atom to viewMode

7996f18

rename embed mode components to simple view

2a55e40

more renaming

a83e8a0

ifsimicoded mentioned this pull request Dec 9, 2025

fix: embed mode follow on PR #1946

Closed

ifsimicoded marked this pull request as ready for review December 9, 2025 03:24

ifsimicoded commented Dec 9, 2025

View reviewed changes

vgeorge self-requested a review December 9, 2025 14:36

vgeorge requested changes Dec 9, 2025

View reviewed changes

ifsimicoded added 3 commits December 9, 2025 07:32

make viewmode type

f1940b4

PR feedback - #1950 (comment)

438ecc2

PR feedback - #1950 (review)

832109b

ifsimicoded requested a review from vgeorge December 9, 2025 16:28

ifsimicoded commented Dec 9, 2025

View reviewed changes

ifsimicoded added 2 commits December 9, 2025 08:35

rename simple component for consistency

97e1a3b

rename for consistency - missed on

3d42ac6

ifsimicoded requested review from ram-sharan25 and slesaad December 9, 2025 17:42

vgeorge approved these changes Dec 9, 2025

View reviewed changes

ifsimicoded commented Dec 9, 2025

View reviewed changes

JS Doc updates

bb2186f

ifsimicoded merged commit 827979f into main Dec 9, 2025
10 checks passed

ifsimicoded deleted the fix/1929/follow_on_fixes_to_PR1922 branch December 9, 2025 21:17

ifsimicoded mentioned this pull request Dec 9, 2025

docs: add JSDoc comments to E&A embed mode implementation #1931

Closed

vgeorge mentioned this pull request Dec 10, 2025

docs: add ADR 006 documenting VEDA2 architecture refactor #1952

Merged

		@@ -0,0 +1,148 @@
		import React, { useEffect, useState } from 'react';

Conversation

ifsimicoded commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Changes

Validation / Testing

Uh oh!

netlify bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for veda-ui ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vgeorge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ifsimicoded commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vgeorge left a comment

Choose a reason for hiding this comment

Uh oh!

ifsimicoded commented Dec 9, 2025

Uh oh!

ifsimicoded Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ifsimicoded commented Dec 8, 2025 •

edited

Loading

netlify bot commented Dec 8, 2025 •

edited

Loading

ifsimicoded commented Dec 9, 2025 •

edited

Loading

ifsimicoded Dec 9, 2025 •

edited

Loading