Skip to content

Conversation

@bmartel
Copy link
Contributor

@bmartel bmartel commented Oct 10, 2025

This pull request refactors the modal implementation across the Label Studio frontend codebase to use the standardized modal components and APIs from @humansignal/ui. The changes remove custom modal logic, consolidate modal usage, and ensure all modals are wrapped with the necessary application providers for consistency and maintainability. This improves code reuse, simplifies future updates, and enhances backward compatibility.

Modal System Refactor and Standardization

  • Migrated all modal-related logic in Modal.jsx, ModalPopup.jsx, and related files to use the standardized modal API from @humansignal/ui, removing custom modal rendering and lifecycle management. (web/apps/labelstudio/src/components/Modal/Modal.jsx [1] web/apps/labelstudio/src/components/Modal/ModalPopup.jsx [2] web/libs/datamanager/src/components/Common/Modal/Modal.jsx [3]
  • Introduced new wrapper modules (Modal.tsx, ModalPopup.tsx) that automatically inject Label Studio-specific providers (like ApiProvider, ConfigProvider, etc.) into every modal for backward compatibility and seamless integration. (web/apps/labelstudio/src/components/Modal/Modal.tsx [1] web/apps/labelstudio/src/components/Modal/ModalPopup.tsx [2]

Codebase Cleanup and Consistency

  • Removed the custom modal styles in Modal.scss to rely on the styles from @humansignal/ui, ensuring consistent appearance and reducing duplicated CSS. (web/apps/labelstudio/src/components/Modal/Modal.scss web/apps/labelstudio/src/components/Modal/Modal.scssL1-L160)
  • Updated all modal usages and imports in downstream modules (such as StorageProviderForm, AccountSettings, and JWT Token sections) to use the new modal API from @humansignal/ui, eliminating legacy imports and ensuring consistent provider injection. (web/libs/app-common/src/blocks/StorageProviderForm/index.tsx [1] [2]; web/libs/app-common/src/pages/AccountSettings/hooks/useHotkeys.ts [3]; web/libs/app-common/src/pages/AccountSettings/sections/Hotkeys/Help.tsx [4] [5]; web/libs/app-common/src/pages/AccountSettings/sections/PersonalJWTToken.tsx [6]

These changes make modal usage more robust, easier to maintain, and aligned with the shared UI library, reducing technical debt and improving developer experience.

@bmartel bmartel requested review from a team, hlomzik and nick-skriabin as code owners October 10, 2025 17:20
@netlify
Copy link

netlify bot commented Oct 10, 2025

Deploy Preview for label-studio-playground ready!

Name Link
🔨 Latest commit 11dcbba
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-playground/deploys/68f14e792acf150008cead85
😎 Deploy Preview https://deploy-preview-8626--label-studio-playground.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 project configuration.

@github-actions github-actions bot added the chore label Oct 10, 2025
@codecov
Copy link

codecov bot commented Oct 10, 2025

Codecov Report

❌ Patch coverage is 82.35294% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.10%. Comparing base (fed66c4) to head (11dcbba).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
web/libs/ui/src/lib/modal/ModalPopup.tsx 82.25% 22 Missing ⚠️
web/libs/ui/src/lib/modal/Modal.tsx 68.33% 19 Missing ⚠️
web/libs/core/src/lib/utils/bem.tsx 92.13% 7 Missing ⚠️
web/libs/ui/src/lib/modal/ModalCloseButton.tsx 25.00% 3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (fed66c4) and HEAD (11dcbba). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (fed66c4) HEAD (11dcbba)
pytests 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8626      +/-   ##
===========================================
- Coverage    66.72%   60.10%   -6.63%     
===========================================
  Files          786      554     -232     
  Lines        60312    38855   -21457     
  Branches     10255    10290      +35     
===========================================
- Hits         40242    23352   -16890     
+ Misses       20067    15500    -4567     
  Partials         3        3              
Flag Coverage Δ
lsf-e2e 54.38% <82.35%> (+0.81%) ⬆️
lsf-integration 50.79% <35.29%> (-0.08%) ⬇️
lsf-unit 8.40% <100.00%> (-0.19%) ⬇️
pytests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@netlify
Copy link

netlify bot commented Oct 10, 2025

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

Name Link
🔨 Latest commit 11dcbba
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-docs-new-theme/deploys/68f14e789dbb450008dee3af

@netlify
Copy link

netlify bot commented Oct 10, 2025

Deploy Preview for label-studio-storybook ready!

Name Link
🔨 Latest commit 11dcbba
🔍 Latest deploy log https://app.netlify.com/projects/label-studio-storybook/deploys/68f14e788308570008e502b8
😎 Deploy Preview https://deploy-preview-8626--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 project configuration.

@netlify
Copy link

netlify bot commented Oct 10, 2025

Deploy Preview for heartex-docs canceled.

Name Link
🔨 Latest commit 11dcbba
🔍 Latest deploy log https://app.netlify.com/projects/heartex-docs/deploys/68f14e78edddfd000874ec62

Copy link
Contributor

@nass600 nass600 left a comment

Choose a reason for hiding this comment

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

Great push 👏 . Other than trying to avoid the multiple modal definitions on top of core for the libraries looks amazing

@robot-ci-heartex robot-ci-heartex merged commit b14581e into develop Oct 17, 2025
45 of 46 checks passed
@robot-ci-heartex robot-ci-heartex deleted the fb-fit-773 branch October 17, 2025 02:40
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.

6 participants