Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

fix: LEAP-523: Return region's text to item in a list #1674

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

hlomzik
Copy link
Collaborator

@hlomzik hlomzik commented Jan 22, 2024

New Outliner dropped this important info from region items in the list.
Returning it back.

PR fulfills these requirements

  • Tests for the changes have been added/updated
  • Docs have been added/updated
  • Best efforts were made to ensure docs/code are concise and coherent (checked for spelling/grammatical errors, commented out code, debug logs etc.)
  • Self-reviewed and ran all changes on a local instance

Describe the reason for change

We need to remove flags for Outliner and NEW UI, enabling it for everyone. To do this smoothly new UI should be very similar to old ones, and that's one of the most important differences.

What feature flags were used to cover this change?

Outliner ff_front_1170_outliner_030222_short
New UI fflag_feat_front_dev_3873_labeling_ui_improvements_short

This change affects (describe how if yes)

  • Performance
  • Security — a little, onn of dangerouslySetInnerHTML removed
  • UX — region text is always visible in the list for all items

Does this PR introduce a breaking change?

  • Yes, and covered entirely by feature flag(s)
  • Yes, and covered partially by feature flag(s)
  • No
  • Not sure (briefly explain the situation below)

What level of testing was included in the change?

  • e2e (codecept)
  • integration (cypress)
  • unit (jest)

New Outliner left this important info from region items in the list.
Returning it back.
We don't need to output it via dangerouslySetInnerHTML,
it's not safe and that's just a text, not an html.
We had different versions at different places,
assuming that it's always a polygon, but it can be a brush.
@hlomzik
Copy link
Collaborator Author

hlomzik commented Jan 22, 2024

/git merge master

Workflow run
Successfully pushed new changes:
Merge remote-tracking branch 'origin/master' into fb-leap-523/text-in-region-list (e1b262a)

@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (c8886fb) 64.60% compared to head (e1b262a) 64.60%.

Files Patch % Lines
.../components/SidePanels/DetailsPanel/RegionItem.tsx 50.00% 1 Missing ⚠️
...mponents/SidePanels/OutlinerPanel/OutlinerTree.tsx 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1674      +/-   ##
==========================================
- Coverage   64.60%   64.60%   -0.01%     
==========================================
  Files         443      443              
  Lines       28730    28734       +4     
  Branches     7526     7530       +4     
==========================================
+ Hits        18561    18563       +2     
- Misses      10169    10171       +2     

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

Co-authored-by: Sergey <[email protected]>
@hlomzik hlomzik merged commit e9c9552 into master Jan 26, 2024
14 checks passed
@hlomzik hlomzik deleted the fb-leap-523/text-in-region-list branch January 26, 2024 14:19
MasherJames pushed a commit to HelloPareto/label-studio-frontend that referenced this pull request Feb 29, 2024
* fix: LEAP-523: Return region's text to item in a list

New Outliner left this important info from region items in the list.
Returning it back.

* Simplify text output in Region Info panel

We don't need to output it via dangerouslySetInnerHTML,
it's not safe and that's just a text, not an html.

* Make "Incomplete polygon/other type" consistent

We had different versions at different places,
assuming that it's always a polygon, but it can be a brush.

* Add test for New UI and regions' texts

* Fix it for different grouppings

With groupping by Label or Tool `item` is undefined

* Remove accidential Scenario.only

* Fix excess symbol

Co-authored-by: Sergey <[email protected]>

---------

Co-authored-by: hlomzik <[email protected]>
Co-authored-by: Sergey <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants