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

CC-163 Add annotation color as a tool in the layer's render tab #53

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

aranega
Copy link
Member

@aranega aranega commented Nov 15, 2024

Summary

This PR moves the widget for color management for local annotation from the annotation tab to the rendering tab, improving UI organization and aligning widget placement with its functionality.

Additionally, this PR fixes an issue in the removeJsonString function to handle tool keys, resolving inconsistencies caused by differences in the toJSON() method across Tool and LegacyTool classes.

Motivation

Relocating the widget enhances usability by grouping it with related options, reducing user confusion.

User Interaction

Widget Relocation: The widget previously located in the state tab is now found in the redering tab. This change simplifies navigation for users working with segment-related settings.
shadercolor

To notify the user that the default color widget moved to the "rendering tab", a new icon with a question mark is displayed in the local-annotation layer, next to the buttons that allows to create new annotations, with a tooltip stating what are the buttons used for, and where is located the color control.

Implementation

Widget Relocation:
Code Changes: Updates in layer/annotation/index.ts moved the initialization and rendering logic of the widget to the redering tab.

removeJsonString Fix:
Problem: The toJSON() method for Tool and LegacyTool behaves differently. For LayerTool (inheriting from Tool), toJSON() returns a jsonId value, causing keys in localToolBinder to include quotes (e.g., "mykey" instead of mykey). This created mismatches when using removeJsonString(), as it required explicit JSON.stringify() calls.
Solution: The fix first attempts to locate the key directly. If it is not found, it falls back to checking the JSON.stringify() version. This approach ensures compatibility with both Tool and LegacyTool implementations.

When a key is inserted in the localToolBinder, the "toJSON()" method is
called on the tool. The JSON retrieved from this call is stringyfied and
used as key. The "toJSON()" method acts differently on Tool and
LegacyTool. When a LayerTool (inheriting from Tool) is created, the
"jsonId" is returned by "toJSON()", the strinfigication of this value
leads then to insert in the localToolBinder map "mykey" (with quotes) instead of
simply mykey. This result in the need to explicitally use
"JSON.stringify(...)" on a key before calling "removeJsonString(...)", which
is not ideal.
This patch on "removeJsonString" searches first for the key and
fallsback on the "JSON.stringify(...)" if the key cannot be found in the
first place.
@aranega aranega requested a review from seankmartin November 15, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants