Add opacity as a top-level property for all layers #879

nakul-py · 2025-08-18T14:00:30Z

Description

This PR adds opacity as a generic property of all JGIS layers, defined once in the schema packages/schema/src/schema/project/jgis.json.

Now opacity is the exact same type of property as visible

Screencast.From.2025-08-19.21-28-23.mp4

Closes #829

Checklist

PR has a descriptive title and content.
PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
PR has one of the labels: documentation, bug, enhancement, feature, maintenance
Checks are passing.
Failing lint checks can be resolved with:
- pre-commit run --all-files
- jlpm run lint

📚 Documentation preview: https://jupytergis--879.org.readthedocs.build/en/879/
💡 JupyterLite preview: https://jupytergis--879.org.readthedocs.build/en/879/lite

github-actions · 2025-08-18T14:00:38Z

👈 Launch a Binder on branch nakul-py/jupytergis/fix%23829

for more information, see https://pre-commit.ci

github-actions · 2025-08-18T14:13:46Z

Integration tests report: appsharing.space

nakul-py · 2025-08-20T04:00:30Z

bot please update snapshots!!!

martinRenou

Thanks a lot for starting this!

Since this is a big schema change, we should probably bump the schema version and have some backward compatibility here https://github.com/geojupyter/jupytergis/blob/main/python/jupytergis_core/jupytergis_core/jgis_ydoc.py#L56

Something like: if the schema version is the old one (when opacity was in the layer parameters) then patch valueDict['layers'] there accordingly by moving the opacity attribute out of the parameters.

This will allow us to continue opening the old file format without error.

for more information, see https://pre-commit.ci

nakul-py · 2025-08-27T04:27:42Z

python/jupytergis_core/jupytergis_core/jgis_ydoc.py

+        if file_version < Version("0.8.1"):
+            for _layer_id, layer in valueDict.get("layers", {}).items():


@martinRenou currently we are using the latest JupyterGIS version 0.8.1 to update the schema. But I am wondering if anyone has an old version, how could we handle that situation? I also use SCHEMA_VERSION instead of latest, but it didn't work.

mfisher87

Thanks for your work on this, Nakul! Haven't had time to go deep on this yet, but some questions immediately came to mind. My biggest thought right now is that we should push as much of this as possible into the sharedModel API, as it seems to not be robust enough to support what we're trying to do. Specifically, I think we need the ability to deep-merge state updates (and a DeepPartial generic type to support that).

i.e.

  // new update method
  updateObject(
    id: string,
    value: DeepPartial<IJGISLayer> | DeepPartial<IJGISSource>,
  ) { 
    // DEEP merge `value` with the current state of object.
  }

// ...

// updated call would _only_ change the specified properties
this.props.model.sharedModel.updateObject(myLayerId, {
  opacity: true,
  parameters: {
    symbologyState: {
      colorRamp: "viridis"
    }
  },
});

(or, maybe better, use the updateLayer, updateSource methods for this)

How do you all feel about this? @martinRenou @arjxn-py @gjmooney more thoughts on this in a comment below.

mfisher87 · 2025-08-28T19:59:37Z

packages/base/src/formbuilder/creationform.tsx

      layerSchema['required'] = ['name', ...layerSchema['required']];
      layerSchema['properties'] = {
        name: { type: 'string', description: 'The name of the layer' },
+        opacity: {


Do we need this? It should already be part of the object, right?

Do we need this?

I think we need.

mfisher87 · 2025-08-28T20:01:57Z

packages/base/src/formbuilder/editform.tsx

+            maximum: 1,
+          },
+        };
+      }


We can omit this since it's defined in the jsonSchema, right?

I think we also need this because if we remove this, then how would we add this to layerschema?
@mfisher87 if you have any thoughts about how to add opacity to formschemaregistry, you can share :)

mfisher87 · 2025-08-28T20:07:48Z

packages/base/src/dialogs/layerBrowserDialog.tsx

        source: sourceId,
      },
      visible: true,
+      opacity: 1.0,


If you want, we can omit this since the default is 1

mfisher87 · 2025-08-28T20:32:56Z

packages/base/src/formbuilder/editform.tsx

+                    this.props.layer,
+                    params,
+                  );
+                }


The fact that a change like this is necessary makes me wonder if we're on the wrong path (I'm not saying your approach is wrong, more that our architecture needs work ;) ) with this change. We have an API for updating layer/source parameters, and moving opacity out of the parameters key is causing us to work around that API. That makes this change much more extensive than I expected. Perhaps the right path here is to update the sharedModel API to support more selective updates.

Perhaps the updateObjectParameters method could be updated to look like this?

updateObject( id: string, value: Partial<IJGISLayer> | Partial<IJGISSource>, ) { // DEEP merge `value` with the current state of object. }

Perhaps it's making our life a bit more complicated to have updateObjectParameters capable of acting on layers or sources, and we should instead have updateLayer, updateSource, etc. (these already exist, but they require you to pass a full layer object in, and replace it in the model) as the sole API method for doing updates of these nested objects?

@mfisher87 I let you have a look at 6459f70 (#879) if it aligns with your idea

for more information, see https://pre-commit.ci

arjxn-py · 2025-09-11T16:10:37Z

Bot please update snapshots

arjxn-py

This is looking good now.
Will wait for @martinRenou & @mfisher87 to merge

arjxn-py · 2025-11-25T16:57:22Z

@nakul-py can you try to manually update the conflicting snapshots? Then we can merge this

mfisher87 · 2025-11-25T17:33:30Z

@arjxn-py any thoughts on this idea? #879 (comment)

nakul-py · 2025-11-26T06:51:18Z

Hey @mfisher87 , I already implemented your idea in this commit 6459f70.

mfisher87 · 2025-11-26T18:12:18Z

packages/schema/src/doc.ts

-      layer.parameters = {
-        ...layer.parameters,
-        ...value,
+      const layerValue = value as Partial<IJGISLayer>;


Is there a way to use a typeguard here instead of a cast? I suppose the only way to do that would be to look at the structure of the object, and that may not make sense / be readable.

There is not any reliable way to use a type guard here because Partial<IJGISLayer> and Partial<IJGISSource> don’t have any guaranteed structural differences. Both interfaces share name, type, and parameters, and the fields that do differ (visible, opacity, filters) are optional. That means a structural type guard would either be unreliable or too hard to maintain.

mfisher87 · 2025-11-26T18:13:28Z

Hey @mfisher87 , I already implemented your idea in this commit 6459f70.

Oh, nice! Sorry I didn't check the source code first :D

Though this is a deep change, and I'm wondering if we have consensus that it's a good idea? @martinRenou @arjxn-py ?

Schema: add opacity as a top-level layer property alongside visible

0301580

[pre-commit.ci] auto fixes from pre-commit.com hooks

4500bfa

for more information, see https://pre-commit.ci

lint fix

d0973ff

arjxn-py added the enhancement New feature or request label Aug 18, 2025

fixing python tests

d2aa1ef

nakul-py marked this pull request as draft August 19, 2025 04:00

nakul-py added 3 commits August 19, 2025 14:39

updating opacity

faed622

Merge branch 'main' into fix#829

b00712c

updating example files and fixing pytest.

582ee85

nakul-py force-pushed the fix#829 branch from 2d835b0 to 582ee85 Compare August 19, 2025 16:41

nakul-py marked this pull request as ready for review August 19, 2025 16:44

nakul-py added 5 commits August 20, 2025 09:32

Merge branch 'main' into fix#829

f815209

Merge branch 'main' into fix#829

f021904

fixing ui-tests

e2d445a

Merge branch 'main' into fix#829

b44011b

Merge branch 'main' into fix#829

e1616f0

martinRenou requested changes Aug 26, 2025

View reviewed changes

nakul-py and others added 3 commits August 26, 2025 20:48

schema update

8442199

[pre-commit.ci] auto fixes from pre-commit.com hooks

1b10c73

for more information, see https://pre-commit.ci

lint fix

7484e02

nakul-py requested a review from martinRenou August 26, 2025 15:25

nakul-py commented Aug 27, 2025

View reviewed changes

mfisher87 reviewed Aug 28, 2025

View reviewed changes

nakul-py and others added 4 commits September 9, 2025 19:15

resolving conflicts

f27c80f

resolving conflicts-1

ba4dc7d

Merge branch 'main' into fix#829

ecd739d

[pre-commit.ci] auto fixes from pre-commit.com hooks

88035e2

for more information, see https://pre-commit.ci

nakul-py added 2 commits September 9, 2025 19:42

updating example files

9c0b37f

using updateObject

6459f70

nakul-py requested a review from mfisher87 September 10, 2025 10:01

Update Playwright Snapshots

35e93e6

arjxn-py closed this Sep 11, 2025

arjxn-py reopened this Sep 11, 2025

arjxn-py approved these changes Sep 11, 2025

View reviewed changes

geojupyter-pr-triage bot added this to JupyterGIS PR triage Sep 12, 2025

mfisher87 reviewed Nov 26, 2025

View reviewed changes

nakul-py force-pushed the fix#829 branch from 5b12234 to 35e93e6 Compare November 28, 2025 07:47

nakul-py added 3 commits November 28, 2025 13:24

resoving conflicts

7ad941b

Merge branch 'main' into fix#829

1f0e297

refactor

80491c9

nakul-py force-pushed the fix#829 branch from cdd0637 to 80491c9 Compare December 6, 2025 07:41

Resolving Conflicts

033c721

		if file_version < Version("0.8.1"):
		for _layer_id, layer in valueDict.get("layers", {}).items():

Add opacity as a top-level property for all layers #879

Are you sure you want to change the base?

Add opacity as a top-level property for all layers #879

Uh oh!

Conversation

nakul-py commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

github-actions bot commented Aug 18, 2025

Uh oh!

github-actions bot commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nakul-py commented Aug 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martinRenou left a comment • 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!

nakul-py Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

mfisher87 left a comment • 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!

mfisher87 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

nakul-py Sep 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!

mfisher87 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

nakul-py Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

mfisher87 Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

mfisher87 Aug 28, 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!

arjxn-py Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

arjxn-py commented Sep 11, 2025

Uh oh!

arjxn-py left a comment

Choose a reason for hiding this comment

Uh oh!

arjxn-py commented Nov 25, 2025

Uh oh!

mfisher87 commented Nov 25, 2025

Uh oh!

nakul-py commented Nov 26, 2025

Uh oh!

mfisher87 Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

nakul-py Nov 28, 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!

mfisher87 commented Nov 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

nakul-py commented Aug 18, 2025 •

edited

Loading

github-actions bot commented Aug 18, 2025 •

edited

Loading

nakul-py commented Aug 20, 2025 •

edited

Loading

martinRenou left a comment •

edited

Loading

mfisher87 left a comment •

edited

Loading

nakul-py Sep 9, 2025 •

edited

Loading

mfisher87 Aug 28, 2025 •

edited

Loading

nakul-py Nov 28, 2025 •

edited

Loading

mfisher87 commented Nov 26, 2025 •

edited

Loading