Multi selection - Githubissues

ffont / freesound-explorer

Visual interface for exploring Freesound content and creating music in a 2-dimensional space

http://labs.freesound.org/fse/

MIT License

19 stars 3 forks source link

Multi selection #39

Closed noVaSon closed 6 years ago

noVaSon commented 6 years ago

This is a small modification of the MapCircle selection behaviour. My goal here was to prepare a batch download (#38), therefore I had to make selection of multiple sounds possible.

I decided to folllow conventions:

The old behaviour is still default with small modifications:

click on a circle and the SoundInfoModal opens
click on the same circle again and the Modal closes new
click on another circle and the old selection is deleted and the Modal opens for the new sound

The new about this PR is:

click on a circle to select it
click on another circle holding the shift key and you can add it to selection
click on a selected circle again, holding shift or not, and the circle is cleared from selection.

Sounds are played only when they are freshly selected, not on deselection.

To mention: I did not manage to implement the batch download, so this feature is not yet of any use.

ffont commented 6 years ago

Hi @noVaSon thanks for all these PRs! They look really awesome. I'll go through them as soon as I can and let you know.

noVaSon commented 6 years ago

Hi @ffont !

Yes, I can hardly wait to run the joined production build! But I know you are very buisy ;)

It is important to apply them in the right order I think. But I assume that you follow it anyway...

I am on a short vacation, take your time.

Greetings from Sweden!

ffont commented 6 years ago

Hi @noVaSon, This PR looks great and is almost ready to merge. Just a couple of comments:

When a selected sound is clicked, it gets unselected regardless of the shit-key being pressed right? That sounds reasonable to me, but is not what I understood from the PR description as it seems to indicate that this only happens when "shift" is pressed.
When a sound is unselected, it should not be played upon click. This might be a matter of preference but i think it feels more natural that sound only gets played when it switches from unselected to selected, but not from selected to unselected.

Do you think these comments make sense?

noVaSon commented 6 years ago

Hi @ffont !

This completely makes sense. Your first comment addressed a wrong description of the PR > updated The second was caused by confusing property management. I was checking this.props.sound.isSelected for the if condition, but it was not defined. I added this note to the new commit in the PR branch. But I will not refactor the code further as I suggested in the commit 62759c2a138b92e6a65fabe27f0cc1119b135144.

Therefore this PR should work now.