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

Fix #317: Added feature for multiple deletion of layers #329

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shivanshmundra
Copy link
Contributor

Fix #317: Added feature for multiple deletion of layers
I added two buttons to include this feature:

  • Select Multiple Layer for choosing layers to delete.
  • Delete Selected Layers for deleting the selected layers.

I am working on the issue of stopping navigation drawer while layers are selected.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.442% when pulling adfd2ac on Shivanshmundra:delete_multiple_layer into 062db16 on Cloud-CV:master.

@Ram81
Copy link
Member

Ram81 commented Mar 14, 2018

@Shivanshmundra can you share a screenshot?

@Shivanshmundra
Copy link
Contributor Author

Yeah Sure:
multiple_layer_deletion

@Ram81 Is this what you wanted?

@Ram81
Copy link
Member

Ram81 commented Mar 14, 2018

@utsavgarg does this design for deleting multiple layers looks good to you?
I have some alternatives in mind if needed

@thatbrguy
Copy link
Contributor

My two cents would be that, as mentioned in #317, I think a "delete mode" button in the actions tab would be a bit neater.

@Shivanshmundra
Copy link
Contributor Author

@thatbrguy I didn't understand clearly what you said #317, like adding a delete mode and then selecting the layers to delete.I think above solution does that just in another way if I am getting you right.
Can you please elaborate what you want?Also if @Ram81 have any good suggestions.

@Ram81
Copy link
Member

Ram81 commented Mar 15, 2018

@Shivanshmundra Delete multiple layers is not a functionality of layer, you can provide a delete multiple layer button in zoo tab under some tab say 'Action' which will allow user to choose multiple layers and then use a delete action to delete all of them at once.
It is not a feature that should be available in the layer parameters pane as per my views. Try to think where is the most suitable place to put the 'Delete multiple layer' action.

@utsavgarg
Copy link
Contributor

@Shivanshmundra Thanks for the effort. The choice of placing the delete multiple layer button in each layer pane is a bit odd to me. Also, when deleting a layer, we need to take care of the parameter calculation, you could discuss this further with @Ram81. I tried to test it in its current state and it doesn't seem to function for a couple of models I imported from the model zoo.

@yashdusing
Copy link
Contributor

I thought of the same approach as @Shivanshmundra did. Isn't it easier to implement a multiple delete button rather than a delete mode button ? What changes are needed to this ?
Also could you tell me about the exact layers it didn't work for

@Ram81
Copy link
Member

Ram81 commented Mar 16, 2018

@yashdusing the delete key approach is better from user perspective.

@thatbrguy
Copy link
Contributor

@Shivanshmundra @yashdusing If both of you want to pursue it, one of you can work on using the delete key for multiple layers, the other can move the delete button to the actions bar. Sorry for the confusion by calling it "delete mode". We just wanted it to be moved where it's common to all layers (Like the actions bar).

The code for parameter count update should be available for single layer deletion, be sure to call it wherever necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants