Skip to content

fix: LEAP-1947: Fix interactive view all FF in image tag case #7387

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Gondragos
Copy link
Collaborator

@Gondragos Gondragos commented Apr 16, 2025

This pull request introduces changes to enable and integrate the FF_DEV_3391 feature flag, which enhances the handling of interactive views and image regions in the annotation tool.

  • fix interactive view all working with MIG approach
  • fix the way Tools Manager works in case of annotation has its own independent tags models tree

Feature Flag Integration:

  • Enabled the FF_DEV_3391 feature flag in flags.json to activate the new interactive view functionality.
  • Updated renderItem in Tree.tsx to include a fallback mechanism (|| el) for rendering image regions when the feature flag is active.

Tool and Manager Enhancements:

  • Added feature flag checks in ToolMixin to dynamically adjust the obj and control getters based on the selected annotation. [1] [2]
  • Introduced a root getter in ToolsManager to provide access to the annotation store's root.

Annotation and Image Model Updates:

  • Enhanced Annotation.js to include imageEntities in the updateIds function for better handling of image-related annotations.
  • Modified Image.js to:
    • Append annotation IDs to image entity IDs for uniqueness.
    • Skip tool initialization if no annotation is selected (that means that we are in the model tree not related to the annotations at all but to the annotation store)

- fix interactive view all working with MIG approach
- fix the way Tools Manager works in case of annotation has it's own independent tags models tree
@github-actions github-actions bot added the fix label Apr 16, 2025
Copy link

netlify bot commented Apr 16, 2025

Deploy Preview for label-studio-storybook ready!

Name Link
🔨 Latest commit 03988e9
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-storybook/deploys/6806587e2f0cc8000866d528
😎 Deploy Preview https://deploy-preview-7387--label-studio-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 16, 2025

Deploy Preview for label-studio-docs-new-theme canceled.

Name Link
🔨 Latest commit 03988e9
🔍 Latest deploy log https://app.netlify.com/sites/label-studio-docs-new-theme/deploys/6806587ea290150008ad3ce6

Copy link

netlify bot commented Apr 16, 2025

Deploy Preview for heartex-docs canceled.

Name Link
🔨 Latest commit 03988e9
🔍 Latest deploy log https://app.netlify.com/sites/heartex-docs/deploys/6806587ef1da4a0008671897

@Gondragos
Copy link
Collaborator Author

Gondragos commented Apr 21, 2025

/git merge

Workflow run
Successfully merged: delete mode 100644 web/dist/apps/labelstudio/version.json

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issues with interactive view rendering when working with image tags by integrating the FF_DEV_3391 feature flag. It also updates various modules to adjust tool manager behavior and annotation IDs for image-related entities.

  • Enabled the FF_DEV_3391 feature flag in multiple files to conditionally change the rendering and tool initialization logic.
  • Updates include modifications to ToolsManager, Tool mixins, and Annotation update functions to ensure IDs and tool references are handled correctly.
  • Adjusted the Tree rendering logic to provide a fallback for image region rendering.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
web/libs/editor/src/tools/Manager.js Added a new getter “root” intended to expose the annotation store’s root.
web/libs/editor/src/tags/object/Image/Image.js Enhanced imageEntities ID generation and introduced a feature flag check in afterAttach.
web/libs/editor/src/stores/Annotation/Annotation.js Extended updateIds to include imageEntities for consistency.
web/libs/editor/src/mixins/ToolManagerMixin.js Added feature flag check in afterAttach to skip tool initialization when not applicable.
web/libs/editor/src/mixins/Tool.js Updated obj and control getters to use the feature flag for annotation selection.
web/libs/editor/src/core/Tree.tsx Modified rendering to use a fallback for image regions via a feature flag.
Files not reviewed (1)
  • web/libs/editor/src/core/feature-flags/flags.json: Language not supported
Comments suppressed due to low confidence (1)

web/libs/editor/src/tools/Manager.js:47

  • The getter 'root' references an undefined variable 'root'. Consider using a properly defined reference, such as getRoot(this), or otherwise ensure that the root context is correctly passed.
return root;

Copy link
Collaborator

@hlomzik hlomzik left a comment

Choose a reason for hiding this comment

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

Add more comments please around views — what and how are you trying to retrieve.
PR itself seems good, just would be better to fully understand what's going on.

Comment on lines +572 to +576
.volatile((self) => {
return {
manager: null,
};
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually use simpler version

Suggested change
.volatile((self) => {
return {
manager: null,
};
})
.volatile((self) => ({
manager: null,
}))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants