Skip to content

Add custom UI for new in situ UV-Vis block #1246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 14, 2025

Conversation

be-smith
Copy link
Contributor

@be-smith be-smith commented Jun 23, 2025

Adds a custom component for the new in situ UV-vis block that closely mirrors the in situ NMR block.

Copy link

codecov bot commented Jun 23, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.58%. Comparing base (a7ad44c) to head (c3e765f).

Files with missing lines Patch % Lines
pydatalab/src/pydatalab/apps/uvvis/__init__.py 25.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1246   +/-   ##
=======================================
  Coverage   74.57%   74.58%           
=======================================
  Files          66       67    +1     
  Lines        4535     4536    +1     
=======================================
+ Hits         3382     3383    +1     
  Misses       1153     1153           
Files with missing lines Coverage Δ
pydatalab/src/pydatalab/apps/uvvis/utils.py 100.00% <100.00%> (ø)
pydatalab/src/pydatalab/apps/uvvis/__init__.py 54.09% <25.00%> (-6.47%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

cypress bot commented Jun 23, 2025

datalab    Run #3596

Run Properties:  status check failed Failed #3596  •  git commit 1e1c2356fe ℹ️: Merge c3e765fb31258c5d3e8b4f26e1acfc147371887f into a7ad44c701aca24176caa4c80ef5...
Project datalab
Branch Review bes/uv-vis-reformat-for-plugin-dev
Run status status check failed Failed #3596
Run duration 07m 25s
Commit git commit 1e1c2356fe ℹ️: Merge c3e765fb31258c5d3e8b4f26e1acfc147371887f into a7ad44c701aca24176caa4c80ef5...
Committer Ben Smith
View all properties for this run ↗︎

Test results
Tests that failed  Failures 2
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 384
View all changes introduced in this branch ↗︎

Tests for review

Failed  batchSampleFeature.cy.js • 1 failed test • End-to-end tests (electron)

View Output

Test Artifacts
Batch sample creation > uses the template id, name, date, copyFrom, and components Test Replay Screenshots
Failed  editPage.cy.js • 1 failed test • End-to-end tests (electron)

View Output

Test Artifacts
Edit Page > Clicks the upload buttons and checks that the modals are shown Test Replay Screenshots

@ml-evs
Copy link
Member

ml-evs commented Jul 10, 2025

Can we figure out what is failing here and merge it before datalab-org/datalab-app-plugin-insitu#45?

@be-smith
Copy link
Contributor Author

be-smith commented Jul 10, 2025

It's trying to import UVVisInsituBlock from the plugin, I assume before the plugin exists in its version of datalab-app-insitu-plugin? When I am on my local version on branch bes/uv-vis-reformat-for-plugin-dev for datalab and bes/uv-vis-insitu-merge for datalab-app-insitu-plugin that test passes.

I think we have to merge datalab-org/datalab-app-plugin-insitu#45 this one first

@ml-evs
Copy link
Member

ml-evs commented Jul 10, 2025

Gotcha, will focus my reviewing there!

@ml-evs ml-evs force-pushed the bes/uv-vis-reformat-for-plugin-dev branch from 3fc7191 to d232021 Compare July 10, 2025 17:31
@ml-evs ml-evs changed the title Bes/uv vis reformat for plugin dev Add custom UI for new in situ UV-Vis block Jul 10, 2025
@ml-evs ml-evs marked this pull request as ready for review July 10, 2025 17:52
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @be-smith!

I've just pushed a few of my own tweaks (creating a new FolderSelect component which adds some basic styling to the folder names, and reorganising the input boxes to be a bit more responsive). Perhaps these could also be used for the in situ NMR block @BenjaminCharmes.

I think with one more round of feedback on the block code in the plugin (e.g., setting default values for granularity), this will be really nice, but lets not hold up this PR if the tests still pass.

@ml-evs ml-evs enabled auto-merge (squash) July 14, 2025 00:04
@ml-evs ml-evs added webapp For issues/PRs pertaining to the web interface datablock An issue pertaining to a specific datablock labels Jul 14, 2025
@ml-evs ml-evs merged commit a18f106 into main Jul 14, 2025
25 of 26 checks passed
@ml-evs ml-evs deleted the bes/uv-vis-reformat-for-plugin-dev branch July 14, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datablock An issue pertaining to a specific datablock webapp For issues/PRs pertaining to the web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants