-
Notifications
You must be signed in to change notification settings - Fork 3.1k
refactor: ECHO-417: Complete BEM Migration: Settings, Label, and Tags Components #8729
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
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for label-studio-docs-new-theme canceled.
|
✅ Deploy Preview for heartex-docs canceled.
|
✅ Deploy Preview for label-studio-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
1a9218a to
c86fd60
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8729 +/- ##
============================================
- Coverage 61.39% 51.11% -10.29%
============================================
Files 790 551 -239
Lines 60687 39007 -21680
Branches 10316 10318 +2
============================================
- Hits 37259 19938 -17321
+ Misses 23425 19066 -4359
Partials 3 3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
967a03f to
5a3cce3
Compare
5a3cce3 to
ede9d65
Compare
| <LoadingOutlined /> | ||
| </div> | ||
|
|
||
| // biome-ignore lint/a11y/useIframeTitle: As a result from BEM migration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am looking for suggestions here.
06d6026 to
d76682a
Compare
web/libs/editor/src/tags/control/TextArea/TextAreaRegionView.jsx
Outdated
Show resolved
Hide resolved
Migrated Choices tag component from Block to cn() helper.
- Replaced Block import with cn import
- Replaced <Block name="choices"> with <div className={cn("choices")...}>
- No Elems in this file (simple wrapper for Choice children)
- Preserved ref, mods, and Tree.renderChildren call
- Added type assertion for ref
- No behavior change, equivalent class strings
Choices
Migrated Labels tag component from Block to cn() helper.
- Replaced Block import with cn import
- Replaced <Block name="labels"> with <div className={cn("labels")...}>
- No Elems in this file (simple wrapper for Label children)
- Preserved mods and Tree.renderChildren call
- No behavior change, equivalent class strings
Migrated Label component from Block/Elem to cn() helper.
- Replaced Block/Elem imports with cn import
- Replaced <Block name="label" tag="span"> with <span className={cn("label")...}>
- Replaced <Elem tag="span" name="text"> with <span className={cn("label").elem("text")...}>
- Replaced <Elem tag="span" name="hotkey"> with <span className={cn("label").elem("hotkey")...}>
- Preserved ref, mods, mix, style, onClick, and all props
- No behavior change, equivalent class strings
label
Migrated Audio tag component from Block to cn() helper.
- Replaced Block import with cn import
- Replaced <Block name="audio-tag"> with <div className={cn("audio-tag")...}>
- No Elems in this file (simple wrapper for waveform and controls)
- No behavior change, equivalent class strings
Migrated RichText tag component from Block/Elem to cn() helper.
- Removed Block/Elem from imports (cn was already imported)
- Replaced <Block name="richtext" tag={ObjectTag}> with <ObjectTag className={cn("richtext")...}>
- Replaced <Elem name="container"> with <div className={cn("richtext").elem("container")...}>
- Replaced <Elem name="loading"> with <div className={cn("richtext").elem("loading")...}>
- Replaced <Elem name="iframe" tag="iframe"> with <iframe className={cn("richtext").elem("iframe")...}>
- Preserved refs, mods, className="htx-richtext" via .mix(), dangerouslySetInnerHTML, and all props
- Added type assertion for loading ref
- Handles both inline and iframe rendering modes
- No behavior change, equivalent class strings
RichText
Migrated Video tag component from Block/Elem to cn() helper.
- Replaced Block/Elem imports with cn import
- Replaced <Block name="video-segmentation"> with <div className={cn("video-segmentation")...}>
- Replaced nested <Block name="video"> with <div className={cn("video")...}>
- Replaced <Elem name="main"> with <div className={cn("video").elem("main")...}>
(Note: uses "video" as block, not "video-segmentation")
- Replaced <Elem name="timeline" tag={Timeline}> with <Timeline className={cn("video-segmentation").elem("timeline")...}>
(Note: uses "video-segmentation" as block - outside nested "video" Block scope)
- Preserved refs for mainContentRef, videoBlockRef, videoContainerRef
- Added type assertions for all refs
- Nested Blocks create correct scope boundaries
- No behavior change, equivalent class strings
HtxVideo
Migrated TextAreaRegionView component from Block/Elem to cn() helper.
- Replaced Block/Elem imports with cn import
- Replaced <Block name="textarea-tag"> with <div className={cn("textarea-tag")...}>
- Replaced <Elem name="item"> with <div className={cn("textarea-tag").elem("item")...}>
- Replaced <Elem name="form" tag={Form}> with <Form className={cn("textarea-tag").elem("form")...}>
- Replaced <Elem name="input" tag={...}> with dynamic Input/TextArea components
- Replaced <Elem name="action" tag={Button}> with <Button className={cn("textarea-tag").elem("action")...}>
- HtxTextAreaResultLine component uses parent Block "textarea-tag" for all Elems
- Dynamic tag (Input vs TextArea) handled with conditional rendering
- Preserved refs, mods, styles, and all props
- Added type assertion for styles with CSS custom properties
- No behavior change, equivalent class strings
Note: Elem "item" under Block "textarea-tag" generates dm-textarea-tag__item
(different from dm-choice__item in Choice.jsx - no CSS conflict)
TextAreaRegionView
fix
Migrated Choice tag component from Block/Elem to cn() helper.
- Replaced Block/Elem imports with cn import, added React import
- Replaced <Block name="choice"> with <div className={cn("choice")...}>
- Replaced <Elem name="item"> with <div className={cn("choice").elem("item")...}>
- Replaced <Elem name="checkbox" component={nameWrapper(...)}> with extracted CheckboxComponent
- Replaced <Elem name="toggle" component={Button}> with <Button className={...}>
- Replaced <Elem name="children"> with <div className={cn("choice").elem("children")...}>
- Extracted nameWrapper result to const for clarity
- Nested Elems within item all use Block "choice"
- Preserved mods, styles, handlers, and all props
- No behavior change, equivalent class strings
Note: Elem "item" under Block "choice" generates dm-choice__item
(different from dm-textarea-tag__item in TextAreaRegionView.jsx - no CSS conflict)
Choice
… test Remove legacy BlockContext.Provider wrapper that is no longer needed. CurrentTask component was migrated to use cn() helper and no longer uses BlockContext internally (no useContext or useBEM calls), so the context wrapper in tests is unnecessary. Changes: - Removed BlockContext import - Removed all BlockContext.Provider wrappers around component renders - Tests still pass - component doesn't consume the context This cleanup allows us to potentially remove BlockContext from bem.ts exports.
…ask test Remove legacy BlockContext.Provider wrapper that is no longer needed. CurrentTask component was migrated to use cn() helper and no longer uses BlockContext internally, so the context wrapper in tests is unnecessary. Changes: - Removed BlockContext import - Removed all BlockContext.Provider wrappers around component renders - Tests still pass - component doesn't consume the context Completes test cleanup for BEM migration.
Remove legacy exports that are no longer used after completing BEM migration. Removed exports: - Block (component) - 0 usages in source code - Elem (component) - 0 usages in source code - BemWithSpecificContext (factory) - 0 usages - useBEM (hook) - 0 usages - BemComponent (type) - 0 usages Kept exports (still in use): - cn (function) - used in ~99 source files - BlockContext (context) - used in 2 test files - CN (type) - return type of cn() - CNTagName (type) - used in Button.tsx and Taxonomy.tsx This completes the BEM migration cleanup. All components now use cn() helper. refactor: remove BlockContext export from bem.ts Remove BlockContext from exports as it's no longer used anywhere in editor. After cleaning up test files to remove unnecessary BlockContext.Provider wrappers, BlockContext has 0 usages in the editor codebase. Final bem.ts exports (minimal set): - cn (function) - primary BEM helper used in ~99 files - CN (type) - return type of cn() - CNTagName (type) - used for dynamic tag types in Button and Taxonomy All legacy Block/Elem system exports now fully removed.
d76682a to
bfa08c7
Compare
| value, | ||
| rows: item.rows, | ||
| className: "is-search", | ||
| className: `is-search ${cn("textarea-tag").elem("input").toClassName()}` , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here @hlomzik was the problem. Obviously :)
Completes the BEM migration by migrating the final 10 components from
BemWithSpecifiContext()wrappers (<Block/>,<Elem/>) to the moderncn()helper function.Changes
10 files migrated:
Cleanup:
Files Changed
Settings
components/Settings/TagSettings/SettingsRenderer.tsxcomponents/Settings/Settings.jsxComponents
components/Label/Label.jsxTags
tags/control/Choices.jsxtags/control/Labels/Labels.jsxtags/control/Choice.jsxtags/control/TextArea/TextAreaRegionView.jsxtags/object/Audio/view.tsxtags/object/RichText/view.jsxtags/object/Video/HtxVideo.jsxMigration Pattern
Before:
After:
What Was Removed from bem.ts
BlockcomponentElemcomponentBemWithSpecificContextfactoryuseBEMhookBlockContextcontextBemComponenttypeExports reduced from 12 to 3:
cn,CN,CNTagNameTesting
Review Recommendation
Please review commit-by-commit - each file is migrated in a separate commit for easy review:
Each commit message documents:
Related PRs