231 display lcms data - #232
Closed
baolanlequang wants to merge 127 commits into
Closed
Conversation
baolanlequang
force-pushed
the
development
branch
from
August 26, 2024 12:03
168ada3 to
1c80817
Compare
baolanlequang
force-pushed
the
231-display-lcms-data
branch
7 times, most recently
from
September 2, 2024 07:30
6e56218 to
ac30aa4
Compare
baolanlequang
force-pushed
the
231-display-lcms-data
branch
7 times, most recently
from
September 12, 2024 14:01
5d7f862 to
8238de9
Compare
feat: display threshold for multiple spectra
Introduce a new, specific action (resetMultiplicity) to reset the multiplicity state to its initial value, preventing any data from leaking between views. - add resetMultiplicity action and handle it in the corresponding reducer - update layer_init.js to call this new action when the spectrum type changes to a non-NMR layout - lint layer_init.js
Bumps [http-proxy-middleware](https://github.com/chimurai/http-proxy-middleware) from 2.0.7 to 2.0.9. - [Release notes](https://github.com/chimurai/http-proxy-middleware/releases) - [Changelog](https://github.com/chimurai/http-proxy-middleware/blob/v2.0.9/CHANGELOG.md) - [Commits](chimurai/http-proxy-middleware@v2.0.7...v2.0.9) --- updated-dependencies: - dependency-name: http-proxy-middleware dependency-version: 2.0.9 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@babel/runtime](https://github.com/babel/babel/tree/HEAD/packages/babel-runtime) from 7.24.8 to 7.27.0. - [Release notes](https://github.com/babel/babel/releases) - [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md) - [Commits](https://github.com/babel/babel/commits/v7.27.0/packages/babel-runtime) --- updated-dependencies: - dependency-name: "@babel/runtime" dependency-version: 7.27.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore: bump version from 1.3.6 to 1.4.0 * ci: bump node engine from 18 to 22 * move babelrc into package.json add @babel/plugin-transform-runtime add postcss-normalize npx update-browserslist-db@latest upgrade interactive - babel - cypress - type
7 tasks
* feat: auto select Reference from solvent metadata when unset Auto select the Reference in the top bar when a matching solvent is found Use a robust matcher on solvent label and name Keep manual choice if a Reference is already set Build a normalized solvent key from feature metadata Score each candidate and choose the best one Select value is now ref.name
Wrapped the buttons in a element to correctly capture hover events and handle refs, aligning them with other working buttons (like Undo/Redo).
Bumps [node-forge](https://github.com/digitalbazaar/forge) from 1.3.1 to 1.3.2. - [Changelog](https://github.com/digitalbazaar/forge/blob/main/CHANGELOG.md) - [Commits](digitalbazaar/forge@v1.3.1...v1.3.2) --- updated-dependencies: - dependency-name: node-forge dependency-version: 1.3.2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-entity building for standalone version
… LCMS multi-entity handling
…e entity information retrieval
Regenerate yarn.lock from merged package.json after resolving lockfile conflict.
+ ignore null children in toolbar collapse count
…with frozen lockfile
This was referenced Jun 23, 2026
Adds src/__tests__/units/pr232_review_blocking.test.tsx, one self-contained suite that reproduces each of the seven blocking issues raised in the review of this PR. Every test asserts the correct (post-fix) behaviour, so it fails on the current branch and turns green once the bug is fixed. - B1 saga_ui.js: peak-add no longer dispatches EDITPEAK.ADD_POSITIVE on non-LCMS layouts (NMR/IR/MS), so peak-add silently no-ops. - B2 integration.js getArea: trapezoidally integrates the cumulative `k` curve instead of returning k(xU)-k(xL); wrong NMR/CV integral. - B3 integration.js getAbsoluteArea: uses cumulative `k` instead of raw `y`. - B4 chem.js Convert2Peak: LC/MS branch discards stored peaks and ignores offset. - B5 multi_focus.js: CV current-density factor double-divides by 100 for mm². - B6 d3_line_rect/index.js: componentDidUpdate dereferences data[0] unguarded. - B7 rect_focus.js drawBar: dereferences tTrEndPts[0] with no length guard. Run: yarn test --watchAll=false src/__tests__/units/pr232_review_blocking.test.tsx
This was referenced Jun 29, 2026
computeCvYScaleFactor divided the current-density factor by 100 twice for areaUnit 'mm²': once via areaInCm2 = safeArea / 100 (correct A/cm²) and again via an explicit `if (areaUnit === 'mm²') factor /= 100`, yielding a factor 100x too small. The B5 review fix removed the duplicate division in the chart (multi_focus.computeYTransformFactor) and the panel (cyclic_voltamery_data.formatCurrent) but missed this third copy, so the value persisted on submit/export disagreed with the displayed numbers. Remove the redundant division so the submit/export factor matches the chart and panel. Export computeCvYScaleFactor and add a regression test asserting 100 mm² is equivalent to 1 cm². refs #232
PiTrem
added a commit
that referenced
this pull request
Jun 29, 2026
…ites The B1–B7 review regression tests lived in one PR-named file (pr232_review_blocking.test.tsx). Move each into the test file that owns the code it guards, matching the repo's subject-based test layout: - B1 peak-add on a non-LCMS layout -> sagas/saga_ui_lcms.test.tsx - B2 getArea on cumulative-k data -> helpers/integration.test.tsx - B3 getAbsoluteArea from raw y -> helpers/integration.test.tsx - B4 Convert2Peak honours stored peaks -> helpers/chem.test.tsx - B7 convertThresEndPts precondition -> helpers/chem.test.tsx - B5 CV current-density chart factor -> components/d3_multi/multi_focus.test.js (new) - B7 drawBar empty-endpoint guard -> components/d3_line_rect.test.js B6 (UV-Vis update-path guard) is dropped: its test only re-implemented the source inline (ViewerLineRect is not exported), so it guarded nothing in the real component. Tracked as a follow-up needing a testable seam. B5's submit/export factor (computeCvYScaleFactor) is already covered in components/cmd_bar/r05_submit_btn.test.js, so it is not duplicated here. No behaviour change — tests only. All moved assertions stay green. refs #232
…ites (#314) The B1–B7 review regression tests lived in one PR-named file (pr232_review_blocking.test.tsx). Move each into the test file that owns the code it guards, matching the repo's subject-based test layout: - B1 peak-add on a non-LCMS layout -> sagas/saga_ui_lcms.test.tsx - B2 getArea on cumulative-k data -> helpers/integration.test.tsx - B3 getAbsoluteArea from raw y -> helpers/integration.test.tsx - B4 Convert2Peak honours stored peaks -> helpers/chem.test.tsx - B7 convertThresEndPts precondition -> helpers/chem.test.tsx - B5 CV current-density chart factor -> components/d3_multi/multi_focus.test.js (new) - B7 drawBar empty-endpoint guard -> components/d3_line_rect.test.js B6 (UV-Vis update-path guard) is dropped: its test only re-implemented the source inline (ViewerLineRect is not exported), so it guarded nothing in the real component. Tracked as a follow-up needing a testable seam. B5's submit/export factor (computeCvYScaleFactor) is already covered in components/cmd_bar/r05_submit_btn.test.js, so it is not duplicated here. No behaviour change — tests only. All moved assertions stay green. refs #232
PiTrem
marked this pull request as ready for review
June 29, 2026 11:54
PiTrem
marked this pull request as draft
June 29, 2026 12:51
Member
|
superseded by #289 |
PiTrem
added a commit
that referenced
this pull request
Jun 29, 2026
PR #289 shipped the LC/MS feature to master at its pre-review state, so the B1–B7 regressions found reviewing #232 are live on master. This applies the fixes that still apply to master's current code: - B1 (saga_ui): restore peak-add for non-LCMS layouts (NMR/IR/MS) — PEAK_ADD dispatched UPDATE_HPLCMS_PEAKS unconditionally, silently swallowing the add - B4 (chem Convert2Peak): honour stored LC/MS feature.peaks (with offset) before recomputing peaks from raw data - B5 (CV current density): apply the mm² area conversion once, not twice (was 100x too small) — fixed in the chart (multi_focus), panel (cyclic_voltamery_data) and submit/export (r05_submit_btn) factors - B6 (d3_line_rect): guard the UV-Vis viewer update path against a featureless feature - B7 (rect_focus): guard drawBar against an empty threshold-endpoint list Not included — B2/B3 (integration getArea/getAbsoluteArea): #303 rewrote the integration helpers on master and they already compute the area from the k-difference / raw y correctly, so that regression is not present there. Adds regression tests for B1, B4, B5 (chart + submit/export), B6, B7. refs #232, #289
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #254