Finishing up remaining TODOs in panel's demo #1144

b-yogesh · 2025-04-01T16:47:53Z

This PR finishes up the TODOs in both the Hist2D demo panels and Spectrum View.

CircularProgress for Histogram 2D panel: After investigating, it seems infeasible within the current Chartlets architecture. Chartlets executes callbacks sequentially and returns all responses together, preventing timely progress updates. To yield responses as they complete would require changes on the Chartlets.js side. If real-time updates are needed, we may need to rethink the callback execution architecture. As an alternative, skeleton component in Chartlets (currently in development) could be used to indicate progress.
Other TODOs: All other TODOs for Histogram 2D and Spectrum View have been completed. However, dynamic resizing of the chart when the side panel width changes requires an update to the VegaChart component in Chartlets (Add dynamic resizing for Vega charts bcdev/chartlets#121).
Spectrum View TODO: Add new State properties xcube-viewer#512
Added exploration modes
- Update: Clear the chart but the current selection if any.
- Add: Current spectrum is added and new point selections will be added as new spectra
- Keeps only last 10 spectra in Add mode(to keep server alive)

Chartlets PR (bcdev/chartlets#122) required to make radio buttons styled for this demo.

In addition, this PR also adds the dataset name, time label and place name to the Hist2D plot as required in Server-side viewer panels: adjustments to layout and functionality xcube-viewer#452 (from Address open issues in viewer demo panel(s) #1134)
Addresses: Address open issues in viewer demo panel(s) #1134

Checklist:

~~Add unit tests and/or doctests in docstrings~~
~~Add docstrings and API docs for any new/modified user-facing classes and functions~~
~~New/modified features documented in docs/source/*~~
Changes documented in CHANGES.md
GitHub CI passes
AppVeyor CI passes
Test coverage remains or increases (target 100%)

codecov · 2025-04-01T17:57:15Z

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 89.28%. Comparing base (4596adf) to head (7595e94).
Report is 82 commits behind head on main.

Files with missing lines	Patch %	Lines
xcube/webapi/viewer/contrib/helpers.py	22.22%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1144      +/-   ##
==========================================
- Coverage   89.31%   89.28%   -0.03%     
==========================================
  Files         279      279              
  Lines       21378    21387       +9     
==========================================
+ Hits        19093    19095       +2     
- Misses       2285     2292       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

clarasb

Review from a user perspective:

Resizing works great !
Demopanels: With no available places, the demo panels show the confusing error message, that the panel is disabled. More context might be useful for a user like: Panel diables due to 0 places available

Visualisation problem in dark mode:
- Sidebar: background colour doesn't adjust to theme, hence the text is not readable
- some text in the horizontal Controlbar above the map and Colourbar Menu do not adjust in font colour, therefore the text is not readable either

The seperation in the horizontal menubar looks nice and organizes the menu bar really well, however, the dropdown menus and buttons on the left side are not leveled with the ones on the right side.

b-yogesh · 2025-04-07T14:55:38Z

Thanks for the user review. Please find my comments below.

2. Demopanels: With no available places, the demo panels show the confusing error message, that the panel is disabled. More context might be useful for a user like: Panel diables due to 0 places available

I decided to just convert the print statements to these messages as is. We plan on aligning all these tabs xcube-dev/xcube-viewer#483 so they all have the same UI design which would then also take care of what is missing for each tab to be useful. Regarding the error message, it is of the user's choice, and as mentioned I used the content from the print statements ~~, but I can change it for now.~~

Visualisation problem in dark mode:

* Sidebar: background colour doesn't adjust to theme, hence the text is not readable

* some text in the horizontal Controlbar above the map and Colourbar Menu do not adjust in font colour, therefore the text is not readable either

I am not able to replicate this issue. Please see the screenshot attached.

The seperation in the horizontal menubar looks nice and organizes the menu bar really well, however, the dropdown menus and buttons on the left side are not leveled with the ones on the right side.

Unable to replicate this as well. See screenshot above!

b-yogesh · 2025-04-08T16:32:11Z

I am not able to replicate this issue. Please see the screenshot attached.

This issue was being caused by using npm link to use local chartlets build for testing and is not an issue created by this PR.
PR to fix npm link issue -> xcube-dev/xcube-viewer#516

CHANGES.md

forman · 2025-05-28T08:02:08Z

examples/serve/panels-demo/demo_panels/panel_histo2d.py

+            "paddingTop": 6,
+            # Since for dynamic resizing we use `container` as width and height for
+            # this chart during updates, it is necessary that we provide the width
+            # and the height here. This is for its parent div.


This is for its parent div.

It is not clear to what "its parent" is referring too: container of the chart or parent of the chart container.

It refers to the container of the chart. Will update the comment.

forman · 2025-05-28T08:02:20Z

examples/serve/panels-demo/demo_panels/panel_histo2d.py

+        chart=None,
+        style={
+            "paddingTop": 6,
+            # Since for dynamic resizing we use `container` as width and height for


What container?

This is a Vega-chart concept which allows the user to set top-level width or height properties to "container" to indicate that the width or height of the plot should be the same as its surrounding container

examples/serve/panels-demo/demo_panels/panel_histo2d.py

examples/serve/panels-demo/demo_panels/panel_spectrum.py

xcube/webapi/viewer/contrib/helpers.py

examples/serve/panels-demo/demo_panels/panel_spectrum.py

Co-authored-by: Norman Fomferra <[email protected]>

…sh-xxx-demo-todos

clarasb

Review from a user perspective:

Spectrum View:

Plot format:
- I prefer the representation of reflectances in point and line format. The bar chart makes it difficult to compare the spectra with each other
Exploration mode:
- The feature works as described in the tooltip, but the name/wording of the features can be misleading.
- Example: The clearing of the plot when switching from add-mode to update-mode. This is not what I would expect as a user.
Spelling/ Wording:
- for coherence: "legend" -> "Legend"
- Suggestion for descriptive text: "Choose an exploration mode and select points to visualize their spectral reflectance across available wavelengths in the Spectrum View."

Histogram View:

The info text is centred. For coherence: position should be left-bound.

b-yogesh · 2025-06-10T11:11:52Z

Thanks for the review Clara.

I have updated the text and fixed the wording as requested.
Regarding other points:

Plot format:

* I prefer the representation of reflectances in point and line format. The bar chart makes it difficult to compare the spectra with each other

This was discussed with Norman to keep the bar plots as line chart does not provide the real information when the distance between the wavelengths is too high.

Example: The clearing of the plot when switching from add-mode to update-mode. This is not what I would expect as a user.

This was done to keep the demo simple.

forman · 2025-06-18T12:50:39Z

Good, so far. Here are my remaining requests:

This looks cluttered. Either use a Select widget or have the mode radios in a row. Make Add the first mode.

Use smaller, secondary typography for the explanation of the panel. Also use markdown widget and don't but the button labels in quotes. Instead use bold letters. Same for the other panels.

Please move the legend to the bottom so right-side space becomes available:

forman

See my comments.

b-yogesh · 2025-06-18T14:24:10Z

This looks cluttered. Either use a Select widget or have the mode radios in a row.

Yes, this was already in row mode, but was waiting for the release from chartlets.

Make Add the first mode.

Done

2. Use smaller, secondary typography for the explanation of the panel. Also use markdown widget and don't but the button labels in quotes. Instead use bold letters. Same for the other panels.

Done

Please move the legend to the bottom so right-side space becomes available:

Done

I assume it was number 3, if you had missed any, let me know.

Screenshots below showing the panels:

I have also created a PR for xcube-viewer to use the latest Chartlets and pin the versions for vega libraries that are defined as peerDeps in chartlets. Pinning is required to avoid dep issue when npm installs the latest version. This might be removed in the future when we dont have dep issues.

Please also review this tiny PR in xcube-viewer: xcube-dev/xcube-viewer#531

forman

Thanks @b-yogesh

b-yogesh added 2 commits April 1, 2025 15:12

add time_label as input

6d1e0de

save work

5dde166

b-yogesh changed the title ~~Yogesh xxx demo todos~~ Finishing up remaining TODOs in panel's demo Apr 2, 2025

b-yogesh self-assigned this Apr 2, 2025

b-yogesh added 9 commits April 2, 2025 10:24

tiny css update

00f7d6d

fix error - NoneType is not subscriptable

5913558

update hist2d

3ac7e0a

save

d3a0134

save work

5324828

save work

0adc680

add new helper func and fix hist demo

7cc71ef

save work

e2a044a

remove style

9f6ed07

b-yogesh removed their assignment Apr 4, 2025

b-yogesh added 6 commits April 4, 2025 15:05

remove print

e3a5adf

use new state properties

19e6ae1

update TODO

0271fd8

remove prints

b7b6e7d

Merge branch 'main' into yogesh-xxx-demo-todos

752ac6e

update CHANGES.md

210d01c

b-yogesh requested review from forman, clarasb and AliceBalfanz April 4, 2025 13:59

b-yogesh marked this pull request as ready for review April 4, 2025 14:00

clarasb reviewed Apr 7, 2025

View reviewed changes

forman requested changes May 28, 2025

View reviewed changes

Merge branch 'main' into yogesh-xxx-demo-todos

9eb80b4

b-yogesh and others added 12 commits May 28, 2025 13:33

Apply suggestions from code review

23de761

Co-authored-by: Norman Fomferra <[email protected]>

Merge remote-tracking branch 'origin/yogesh-xxx-demo-todos' into yoge…

eb25410

…sh-xxx-demo-todos

update hist2d

772ddf2

save work

ad6272a

update demos

0d37405

save work

85bc685

save work

899bf20

save work

adf6be0

more updates to spectrum viewer

095c8b3

remove datamanager and use chart as state instead

cb15f99

update spectrum viewer

7783d92

update spectrum viewer

7bb467a

b-yogesh requested review from forman and clarasb and removed request for AliceBalfanz June 6, 2025 15:35

b-yogesh mentioned this pull request Jun 6, 2025

Apply styles correctly in RadioGroup component bcdev/chartlets#122

Merged

clarasb reviewed Jun 10, 2025

View reviewed changes

address reviewers comments

5b151e8

forman requested changes Jun 18, 2025

View reviewed changes

address reviewers comments

7595e94

b-yogesh requested a review from forman June 18, 2025 14:39

forman approved these changes Jun 19, 2025

View reviewed changes

forman merged commit 4e6305f into main Jun 19, 2025
3 of 5 checks passed

Finishing up remaining TODOs in panel's demo #1144

Finishing up remaining TODOs in panel's demo #1144

Uh oh!

Conversation

b-yogesh commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

clarasb 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!

b-yogesh commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

b-yogesh commented Apr 8, 2025

Uh oh!

Uh oh!

forman May 28, 2025

Choose a reason for hiding this comment

Uh oh!

b-yogesh May 28, 2025

Choose a reason for hiding this comment

Uh oh!

forman May 28, 2025

Choose a reason for hiding this comment

Uh oh!

b-yogesh May 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clarasb left a comment

Choose a reason for hiding this comment

Uh oh!

b-yogesh commented Jun 10, 2025

Uh oh!

forman commented Jun 18, 2025

Uh oh!

forman left a comment

Choose a reason for hiding this comment

Uh oh!

b-yogesh commented Jun 18, 2025

Uh oh!

forman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

b-yogesh commented Apr 1, 2025 •

edited

Loading

codecov bot commented Apr 1, 2025 •

edited

Loading

clarasb left a comment •

edited

Loading

b-yogesh commented Apr 7, 2025 •

edited

Loading