Skip to content
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

Fix animation modal color layer validation #7882

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Jun 14, 2024

This PR fixes animation modal color layer validation. Unfortunately, the animation validation would always try the first color layer and not the selected one.

Steps to test:

  • Enable jobs. (Worker is not required to be running)
  • Create a dataset with two color layers.
    • Copy an existing layer on disc
    • Modify the new layer's properties to be dtype "uint24".
  • Launch the animation modal, select the uint24 color layer, submit --> validation error
  • Launch the animation modal, select the other uint8 color layer, submit --> success

Issues:


(Please delete unneeded items, merge only when none are left open)

@@ -230,7 +230,8 @@ function CreateAnimationModal(props: Props) {
cameraPosition: selectedCameraPosition,
};

if (!validateAnimationOptions(colorLayer, boundingBox, meshes)) return;
const selectedColorLayer = colorLayers.find((layer) => layer.name === selectedColorLayerName)!;
Copy link
Contributor

Choose a reason for hiding this comment

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

line 108 already defines selectedColorLayer, which should be usable here.

cf: line 108: const selectedColorLayer = getLayerByName(dataset, selectedColorLayerName);

Copy link
Contributor

Choose a reason for hiding this comment

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

Thus, I'd suggest to remove this line

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch. done.

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

LGTM & works 🎉

@hotzenklotz hotzenklotz enabled auto-merge (squash) June 14, 2024 07:52
@hotzenklotz hotzenklotz merged commit f86b4f0 into master Jun 14, 2024
2 checks passed
@hotzenklotz hotzenklotz deleted the fix-animation-modal-validation branch June 14, 2024 08:08
MichaelBuessemeyer added a commit that referenced this pull request Jun 17, 2024
* fix animation modal color layer validation

* changelog

* rename variable

* apply pr feedback

* improve error message of uint24 layers in animation modal

---------

Co-authored-by: Michael Büßemeyer <michael.buessemeyer@student.hpi.de>
aaronkanzer pushed a commit to lincbrain/webknossos that referenced this pull request Jun 17, 2024
* fix animation modal color layer validation

* changelog

* rename variable

* apply pr feedback

* improve error message of uint24 layers in animation modal

---------

Co-authored-by: Michael Büßemeyer <michael.buessemeyer@student.hpi.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants