-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fixing up logic around selecting/deselecting nested labels #13
Conversation
There’s a bug in label studio where if you use labels for conditional labeling (creating nested labels), and don’t deselect the label you’re on before attempting to label something else, the console will throw a bunch of errors and beachball of death, forcing you to reload the page. Fixing this by not allowing users to select a different top-level label if there are sub-labels already applied. Also not allowing users to remove a top-level label if there are sub-labels since that logically doesn't make sense. Issue: https://khanacademy.atlassian.net/browse/DI-1502 Test plan: - deployed to test; see in action here: https://data-labeling-test.khanacademy.org/projects/298/data?tab=537&page=1
<full summary> Issue: <url or "none"> Test plan: <see below>
<full summary> Issue: <url or "none"> Test plan: <see below>
<full summary> Issue: <url or "none"> Test plan: <see below>
src/tags/control/Label.js
Outdated
if ( | ||
labels.selectedLabels.length === 1 && | ||
self.selected && | ||
labels.selectedLabels[0].value == self.value && |
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.
src/tags/control/Label.js
Outdated
!labels.selectedLabels[0].alias && // Hack: we only gave sub-labels aliases | ||
region.labelings.length > 1 | ||
) { | ||
console.log("can't deselect top level label w sub label!") |
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.
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.
LGTM! Thanks for working this one out!
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/tags/control/Label.js
Outdated
!labels.selectedLabels[0].alias && // Hack: we only gave sub-labels aliases | ||
region.labelings.length > 1 | ||
) { | ||
console.log('can\'t deselect top level label w sub label!') |
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.
…083bb0e1d58c941cb5ed55b11612efa095fd at Khan/label-studio-frontend#13 Workflow run: https://github.com/Khan/label-studio/actions/runs/10119334387
Summary:
There’s a bug in label studio where if you use labels for conditional labeling (creating nested labels), and don’t deselect the label you’re on before attempting to label something else, the console will throw a bunch of errors and beachball of death, forcing you to reload the page. Fixing this by not allowing users to select a different top-level label if there are sub-labels already applied. Also not allowing users to remove a top-level label if there are sub-labels since that logically doesn't make sense.
Issue: https://khanacademy.atlassian.net/browse/DI-1502
Test plan: