-
Notifications
You must be signed in to change notification settings - Fork 3.1k
refactor: ECHO-415: BEM Migration in editor #8660
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
Conversation
✅ 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. |
✅ Deploy Preview for label-studio-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #8660 +/- ##
===========================================
- Coverage 61.37% 60.05% -1.33%
===========================================
Files 791 554 -237
Lines 60442 38904 -21538
Branches 10291 10305 +14
===========================================
- Hits 37096 23362 -13734
+ Misses 23343 15539 -7804
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:
|
f5f63f8
to
8d6037e
Compare
Migrated Space component from BemWithSpecifiContext Block wrapper to cn() helper. - Removed BemWithSpecifiContext import and Block destructuring - Replaced <Block name="space"> with <div className={cn("space")...}> - Preserved all mods, mixes, and props in correct order - No behavior change, equivalent class strings
Migrated ViewControls component from BemWithSpecifiContext wrappers to cn() helper. - Removed BemWithSpecifiContext import and Block/Elem destructuring - Replaced <Block name="view-controls"> with <div className={cn("view-controls")...}> - Replaced <Elem name="sort"> with <div className={cn("view-controls").elem("sort")...}> - Replaced <Elem name="label"> with <div className={cn("view-controls").elem("label")...}> - Preserved all mods, handlers, and props - No behavior change, equivalent class strings
Migrated GroundTruth component from BemWithSpecifiContext wrappers to cn() helper. - Removed BemWithSpecifiContext import and Block/Elem destructuring - Replaced <Block name="ground-truth"> with <div className={cn("ground-truth")...}> - Replaced <Elem name="indicator" tag={...}> with dynamic component variable - Extracted IndicatorIcon component selection to const for clarity - Preserved all mods, dynamic tag behavior, and props - No behavior change, equivalent class strings
Migrated TimeBox component from global Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="time-box"> with <div className={cn("time-box")...}> - Replaced <Elem name="input-time" tag="input"> with <input className={cn("time-box").elem("input-time")...}> - Preserved all props, refs, handlers, and mods - No behavior change, equivalent class strings
Migrated TimeDurationControl component from global Block to cn() helper. - Replaced Block import with cn import - Replaced <Block name="timer-duration-control"> with <div className={cn("timer-duration-control")...}> - No mods or mixes to preserve - No behavior change, equivalent class strings
Migrated Pagination component from global Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="pagination"> with <div className={cn("pagination")...}> - Replaced all <Elem> instances (navigation, divider, input, page-indicator, btn, page-size) - Preserved all mods, handlers, hotkeys, and props - No behavior change, equivalent class strings
Migrated Hint component from global Block to cn() helper. - Replaced Block import with cn import - Replaced <Block name="hint" tag="sup"> with <sup className={cn("hint")...}> - Preserved className mix, data attributes, and styles - No behavior change, equivalent class strings
Migrated Button component from global Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import, added createElement - Used createElement for dynamic tag support in button body - Replaced <Block name="button"> with createElement(finalTag, {className: cn("button")...}) - Replaced <Elem name="icon" tag="span"> with <span className={cn("button").elem("icon")...}> - Replaced <Elem name="extra"> with <div className={cn("button").elem("extra")...}> - Replaced <Block name="button-group"> with <div className={cn("button-group")...}> - Preserved all mods, mixes, refs, dynamic tags, and props - No behavior change, equivalent class strings
Migrated Range component from global Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="range"> with <div className={cn("range")...}> - Replaced all <Elem> instances (icon, body, line, range-handle, indicator) - Preserved all mods, styles, event handlers, and props - No behavior change, equivalent class strings
Migrated Icon component from global Block to cn() helper. - Replaced Block import with cn import - Replaced <Block tag="span" name="icon"> with <span className={cn("icon")...}> - Preserved ref and all props - No behavior change, equivalent class strings
Migrated AnnotationButton component from Block/Elem to cn() helper. - Removed Block/Elem from imports (cn was already imported) - Replaced <Block name="annotation-button"> with <div className={cn("annotation-button")...}> - Replaced all <Elem> instances (mainSection, picSection, userpic, main, user, name, entity-id, info, date, icons, icon) - Replaced <Elem tag={Userpic}> with <Userpic className={...}> - Replaced <Elem component={TimeAgo}> with <TimeAgo className={...}> - Preserved all mods, handlers, tooltips, and props - No behavior change, equivalent class strings
Migrated AnnotationsCarousel component from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="annotations-carousel"> with <div className={cn("annotations-carousel")...}> - Replaced <Elem> instances (container, carosel, carousel-controls) - Preserved refs, mods, styles, handlers, and all props - Added type assertions for refs and CSS custom properties - No behavior change, equivalent class strings
Migrated Label component from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn and createElement imports - Used createElement for dynamic tag (div vs label) support - Replaced <Block name="field-label"> with createElement(tagName, {className: cn("field-label")...}) - Replaced <Elem> instances (text, content, description, field) - Preserved ref, mods, styles, and data attributes - No behavior change, equivalent class strings
Migrated Menu component from Block to cn() helper. - Removed Block from imports (cn was already imported) - Replaced <Block tag="ul" name="menu"> with <ul className={cn("menu")...}> - Preserved ref, mods, mix, style, and onClick handler - Menu.Spacer, Menu.Divider, and Menu.Group already use cn() - No behavior change, equivalent class strings
Migrated TaxonomySearch component from Block to cn() helper. - Replaced Block import with cn import - Replaced <Block tag="input"> with <input className={cn("taxonomy-search-input")...}> - Preserved ref, value, handlers, placeholder, and data attributes - Added type assertion for ref - No behavior change, equivalent class strings
Migrated RelationsControls component from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="relation-controls"> with <div className={cn("relation-controls")...}> - Replaced <Elem tag={Button}> with <Button className={cn("relation-controls").elem(...)...}> - Created proper elem names: toggle-visibility and toggle-order - Preserved all mods, handlers, props, and Button functionality - No behavior change, equivalent class strings
Migrated DetailsPanel component from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="details-tab"> with <div className={cn("details-tab")...}> - Replaced <Block name="comments-panel"> with <div className={cn("comments-panel")...}> - Replaced <Block name="relations"> with <div className={cn("relations")...}> - Replaced <Block name="history"> with <div className={cn("history")...}> - Replaced <Block name="info"> with <div className={cn("info")...}> - Replaced all <Elem> instances (section, section-tab, section-head, section-content, view-control) - Preserved all props and structure - No behavior change, equivalent class strings
…ons.jsx Migrated BottomBar and Actions components from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="bottombar"> with <div className={cn("bottombar")...}> - Replaced <Elem name="group"> with <div className={cn("bottombar").elem("group")...}> - Replaced <Elem name="section"> with <div className={cn("bottombar").elem("section")...}> - Preserved mods, styles, and all props - No behavior change, equivalent class strings
Migrated BottomBar Controls component from Block/Elem to cn() helper. - Removed Block/Elem from imports (cn was already imported) - Replaced <Block name="controls"> with <div className={cn("controls")...}> - Replaced <Elem name="skipped-info"> with <div className={cn("controls").elem("skipped-info")...}> - Replaced <Elem name="tooltip-wrapper"> with <div className={cn("controls").elem("tooltip-wrapper")...}> - Preserved all handlers and props - No behavior change, equivalent class strings
Migrated CurrentTask component from Block/Elem to cn() helper. - Replaced Block/Elem imports with cn import - Replaced <Block name="current-task"> with <div className={cn("current-task")...}> - Replaced <Elem name="section"> with <div className={cn("bottombar").elem("section")...}> - Replaced <Elem> instances (task-id, task-count, history-controls) - Preserved all mods and props - No behavior change, equivalent class strings
…age.jsx Migrated VideoCanvas and Image components from Block/Elem to cn() helper. VideoCanvas.tsx: - Replaced <Block name="video-canvas"> with <div className={cn("video-canvas")...}> - Replaced <Elem name="loading"> with <div className={cn("video-canvas").elem("loading")...}> - Replaced <Block name="spinner"> with <div className={cn("spinner")...}> - Replaced <Elem name="view"> with <div className={cn("video-canvas").elem("view")...}> - Replaced <Elem name="buffering"> with <div className={cn("video-canvas").elem("buffering")...}> Image.jsx: - Replaced <Block name="image"> with <div className={cn("image")...}> - Replaced <Block name="image-progress"> with <div className={cn("image-progress")...}> - Replaced <Elem name="message"> with <div className={cn("image-progress").elem("message")...}> - Replaced <Elem tag="progress" name="bar"> with <progress className={cn("image-progress").elem("bar")...}> Preserved refs, styles, handlers, ARIA attributes, and all props. No behavior change, equivalent class strings.
… App.jsx Migrated AnnotationTabs and App components from Block/Elem to cn() helper. AnnotationTabs.jsx: - Replaced <Block name="entity-tab"> with <div className={cn("entity-tab")...}> - Replaced <Elem tag={Userpic}> with <Userpic className={...}> - Replaced <Elem name="identifier"> with <div className={cn("entity-tab").elem("identifier")...}> - Replaced <Elem tag={IconStar}> with <IconStar className={...}> - Replaced <Elem tag={IconBan}> with <IconBan className={...}> App.jsx: - Replaced <Block name="editor"> with <div className={cn("editor")...}> - Replaced <Block name="main-view"> with <div className={cn("main-view")...}> - Replaced <Block name="main-content"> with <div className={cn("main-content")...}> - Replaced <Block name="wrapper"> with <div className={cn("wrapper")...}> - Replaced <Block name="sub__result"> with <div className={cn("sub__result")...}> - Replaced <Elem name="annotation"> with <div className={cn("main-view").elem("annotation")...}> - Replaced <Elem name="infobar" tag={Space}> with <Space className={cn("main-view").elem("infobar")...}> Preserved refs, mods, styles, handlers, and all props. No behavior change, equivalent class strings.
Add missing type attribute to button element to prevent default form submission behavior and satisfy linter requirements.
Update jest mocks for bem utils to include the cn() helper function that returns a chainable API for BEM class generation (elem, mod, mix, toClassName). Removed Block/Elem component mocks as they are no longer used after migration. This fixes test failures after migrating from Block/Elem components to cn() helper. Files updated: - OutlinerPanel.test.tsx - DetailsPanel.test.tsx
Update test assertions to use text-based queries instead of data-testid queries that relied on the old Block/Elem component mocks. Changes: - Replace screen.getByTestId('block') with screen.getByText() queries - Replace screen.getAllByTestId('elem').find() with direct text queries - Update assertions to check for actual rendered text content This completes the test migration after replacing Block/Elem with cn() helper.
web/libs/editor/src/components/SidePanels/DetailsPanel/RegionEditor.tsx
Outdated
Show resolved
Hide resolved
web/libs/editor/src/components/SidePanels/DetailsPanel/RegionDetails.tsx
Outdated
Show resolved
Hide resolved
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.
Titanic job!
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.
selenium passed, I have a minor observation but beyond that if checks are all green than approved!
/git merge
|
BEM Migration: Replace
BemWithSpecifiContext()
/Block
/Elem
withcn()
HelperSummary
This PR migrates the editor codebase from the legacy
BemWithSpecifiContext()
wrapper components (<Block/>
,<Elem/>
) to the moderncn()
helper function for BEM class name generation.Migration Status:
All remaining files are safe to migrate and will be completed in a follow-up PR.
Migration Approach
Conservative Safety Rules Applied
name
attributes were literal stringsTransformation Patterns
Before (Block):
After (cn):
Before (Elem):
After (cn):
Before (Dynamic Tag with Block):
After (cn with createElement):
Before (Elem with Component Tag):
After (cn with Component):
Files Migrated (62)
Common Components (15)
Core Components (10)
TopBar (6)
BottomBar (4)
Toolbar (3)
SidePanels (13)
TabPanels (3)
Timeline (11)
Core Utilities (2)
Annotation Tabs (2)
Files Remaining (22)
These files were not skipped for safety reasons - they simply haven't been migrated yet and can be completed in a follow-up PR:
Tags Components (7)
Comments Components (10)
Settings (2)
Other (3)
Key Benefits
1. Simplified API
Block
/Elem
componentscn()
function calls - explicit and clearBemWithSpecifiContext()
2. Smaller Bundle
3. Better Type Safety
4. Easier to Read and Maintain
cn("block-name")
easier to find than<Block name="block-name">
5. Consistency
Migration Statistics
Code Changes
Commit History
refactor(bem): replace Block/Elem with cn() in [Filename]
Testing Recommendations
Visual Regression Testing
Functional Testing
Critical User Flows
Areas of Focus
High Complexity Components:
Interactive Components:
Migration Safety Guarantees
✅ No Breaking Changes
cn(block).elem(elem).mod(mod).mix(mix).toClassName()
✅ Code Quality Maintained
✅ Component Behavior Preserved
Technical Details
Class Name Generation Order
The migration maintains the exact order of class generation:
Special Cases Handled
Dynamic Tags: Used
createElement()
for components with dynamic tag namesComponent Tags: Converted to direct component usage with className
Nested Blocks: Each Block creates new scope for its Elems
Refs: Added type assertions where needed
CSS Custom Properties: Added type assertions for style objects
Rollback Plan
Each file has an individual commit, making it easy to:
Revert specific files if issues found:
Cherry-pick fixes to problematic files
Roll back entire migration:
Revert by file pattern:
Files Migrated - Detailed List (62)
Common Components (11)
Core Application (5)
Time Controls (2)
Annotations & Carousel (3)
Current Entity (2)
Annotation Tabs (2)
TopBar (6)
BottomBar (4)
Toolbar (3)
SidePanels Base (2)
OutlinerPanel (4)
DetailsPanel (6)
TabPanels (3)
Timeline (11)
Core & Libraries (2)
Files Remaining - Safe to Migrate (22)
All remaining files use only literal Block/Elem names with no cross-file overlaps. They are safe to migrate and will be completed in a follow-up PR.
Tags/Object Components (3)
Tags/Control Components (4)
Comments Components (10)
Settings (2)
Other (3)
Why No Files Were Skipped
During the migration analysis, all 84 files with Block/Elem usage were verified to be safe for migration:
The 22 remaining files are simply pending migration, not skipped for safety. They follow the same safe patterns and can be migrated using identical transformation rules.
Validation Performed
Pre-Migration Checks
name={variable}
)Post-Migration Verification
Next Steps
Immediate
Follow-Up
BemWithSpecifiContext()
export once all migrations completeRisk Assessment
Risk Level: LOW
Rationale:
Mitigation:
Total Impact: