Skip to content
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

953 remove strand in batch #954

Merged
merged 4 commits into from
Nov 12, 2023
Merged

953 remove strand in batch #954

merged 4 commits into from
Nov 12, 2023

Conversation

rayzhuca
Copy link
Member

Fixes batch delete/set labels and names for domains and strands.

Description

  • Used BatchAction to group together set labels/name.
  • Created get_selected_domains function that returns a BuiltSet<Domain> set, so setting domain names still work if
    user selects with domain tool
  • Renamed some functions like set_substrand_names to set_domain_names.

@rayzhuca rayzhuca added the bug Something isn't working label Oct 24, 2023
@rayzhuca rayzhuca requested a review from dave-doty as a code owner October 24, 2023 17:21
Copy link
Member

@dave-doty dave-doty left a comment

Choose a reason for hiding this comment

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

This is a good start, but it should also work for extensions and loopouts. For example, if I select these two extensions and select "Remove extension name":

image

it should remove both names, but still removes only one.

If you notice, the original function names talked about substrands rather than domains. I named them that way to make the code generally useful for all three types of substrands (Domain, Extension, and Loopout). Although maybe they weren't being used in the latter two cases, but nevertheless there should be some way (without too much copy-pasting of code) to make this work for all three cases.

(strand) => actions.StrandNameSet(name: null, strand: strand),
props.strand,
app.state.ui_state.selectables_store.selected_strands,
'remove strand label'))),
Copy link
Member

Choose a reason for hiding this comment

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

I think this should say "remove strand name" instead of "remove strand label"

@rayzhuca rayzhuca merged commit ced32b2 into dev Nov 12, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants