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

Actions on multiple messages #2273

Closed
5 of 11 tasks
StCyr opened this issue Nov 22, 2019 · 12 comments · Fixed by #2619
Closed
5 of 11 tasks

Actions on multiple messages #2273

StCyr opened this issue Nov 22, 2019 · 12 comments · Fixed by #2619

Comments

@StCyr
Copy link
Collaborator

StCyr commented Nov 22, 2019

Feature Request

I would like to be able to perform actions (eg: delete) on multiple messages at once.

Summary

It looks a bit complicated to implement at first sight. So, I'd like to have some help here :-)

Here's a proposal to start the discussion.

Proposal

What I think is necessary to implement to get this functionality is:

  1. Implement a new store object/array to keep track of the currently selected messages
  2. Be able to divert CTRL+click and SHIFT+click from their regular behaviour in the Envelope/EnvelopList component (=> to update the above-mentioned store object for example)
  3. New component to display in place of the Message/NewMessageDetail/NoMessageSelected components when multiple messages are selected. This component would show the possible actions (inspired by OWA)

Decision

  1. The tracking of the currently selected messages must be implemented with a property of the EnvelopeList component;
  2. Show an "action header" at the top of the EnvelopeList component rather than create a new component to show the possible actions.

TODO

  • create a selection array in the envelopeList component
  • handle ctrl/shift/meta+click events in the envelope component
  • show an action header at the top of the envelopeList component when multiple
  • style the action header better
  • make the header's position fixed
  • implement "mark read/unread" actions
  • implement "delete" action
  • automatically add the currently viewed message to the selection list
  • clear the selection list when needed
  • verify the algorithm used to populate the selection property
  • proper icons for "mark read" and "mark unread"

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@ChristophWurst
Copy link
Member

I'm pretty sure that is at least one existing ticket for this. But I can't find it right now. 😆

IMO the selection is a local state. It's only relevant to the component that renders the list. This shouldn't go into a store.

Generally speaking this functionality isn't specific to this app. It doesn't make sense to have a specific implementation for every app that renders a list. Instead, this should be added to the https://github.com/nextcloud/nextcloud-vue components (nextcloud-libraries/nextcloud-vue#616). And then apps can specify if actions are available for multi select.

👍 on the CTRL+click behavior for power users on a desktop system.

Not sure about the 3). It could also be fine to just keep showing the previously shown message. It definitely shouldn't be the (only) place for the actions, as this element is not visible on mobile.

@StCyr
Copy link
Collaborator Author

StCyr commented Nov 22, 2019

I'm pretty sure that is at least one existing ticket for this. But I can't find it right now. 😆

I didn't find neither. Strange uh?

IMO the selection is a local state. It's only relevant to the component that renders the list. This shouldn't go into a store.

Generally speaking this functionality isn't specific to this app. It doesn't make sense to have a specific implementation for every app that renders a list. Instead, this should be added to the https://github.com/nextcloud/nextcloud-vue components (nextcloud/nextcloud-vue#616). And then apps can specify if actions are available for multi select.

I've hacked a little bit and that might be not too dificult to implement this functionality using HTML classes and DOM selectors:

  1. You ctrl+click on an envelope, it add/remove a "selected" class to/from the envelope.
  2. When clicking on an action, it is applied to all envelope with the "selected" class;

Gotcha:

  1. A simple way to remove the "selected" class from all envelope (that's: reset the multiple selection state) must be implemented
  2. Should the action labels be changed when multiple envelopes are selected?

👍 on the CTRL+click behavior for power users on a desktop system.

Not sure about the 3). It could also be fine to just keep showing the previously shown message. It definitely shouldn't be the (only) place for the actions, as this element is not visible on mobile.

Yeah, it could also be fine.

But, if multiple messages are selected and we let the user perform actions on them using the usual envelope Actions menu, then it must be clear for the user that the action he's about to perform will act on multiple messages; Again, should the action labels be automaticaly changed when multiple envelopes are selected?

@ChristophWurst
Copy link
Member

I didn't find neither. Strange uh?

Yeah 🙈

But, if multiple messages are selected and we let the user perform actions on them using the usual envelope Actions menu, then it must be clear for the user that the action he's about to perform will act on multiple messages; Again, should the action labels be automaticaly changed when multiple envelopes are selected?

That is correct. The individual actions on each envelope should still only affect one envelope. But once you select more than message there could be a new header that appears lile

x message selected ··· <- and that open a actions menu with actions that are applied to all selected messages

@StCyr
Copy link
Collaborator Author

StCyr commented Nov 25, 2019

That is correct. The individual actions on each envelope should still only affect one envelope. But once you select more than message there could be a new header that appears lile

x message selected ··· <- and that open a actions menu with actions that are applied to all selected messages

Nice idea. Is there already a component for such a header? :-)

@StCyr
Copy link
Collaborator Author

StCyr commented Nov 25, 2019

Hey,

I've started working on this here: https://github.com/nextcloud/mail/tree/feature/2273/multi-select

But I'm struggling to have the view to update directly when the user CTRL+Click on envelopes.

I guess it's a reactivity issue but I can't find the solution.

Any idea why the view doesn't update? @ChristophWurst ? Is it because envelope.flags.selected doesn't exist when the envelopes are fectched?

@ChristophWurst
Copy link
Member

Did you use Vue.set(envelope, 'selected', true)? Otherwise yes, this will cause reactivity issues if you just set the property directly and it didn't exist before.

@ChristophWurst
Copy link
Member

but sounds like you're storing the selected information in the store. and as discussed before I would not consider this global state. The selected state is only relevant to the list of envelopes. It should be stored there (simple array of IDs)

@StCyr
Copy link
Collaborator Author

StCyr commented Dec 30, 2019

yeah fixed it :-)

@StCyr
Copy link
Collaborator Author

StCyr commented Jan 3, 2020

hey @ChristophWurst,

I've now something working properly with the selected state stored in EnvelopeList.

Now, this selected state should be reset everytime the user makes a "non-selective" action (eg: the user starts making a selection, but then changes his mind and click on the "+ New message" button).

Do you have any idea how to achieve this?

@ChristophWurst
Copy link
Member

Do you have any idea how to achieve this?

I would only clear that state when the view is shown. E.g. with the mounted event in Vue or similar. When the users decides to write a new message the list is still shows, so I'd argue we shouldn't clear the selection.

@laurentiu2
Copy link

#485

@small1
Copy link

small1 commented Jan 28, 2020

I am going to see if i can open a small bounty for this. This feature is badly needed :)

@StCyr StCyr mentioned this issue Feb 5, 2020
2 tasks
StCyr added a commit that referenced this issue Mar 6, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Mar 6, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Mar 6, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Mar 25, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Mar 26, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Mar 31, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Apr 1, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue Apr 14, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue May 5, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue May 5, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
StCyr added a commit that referenced this issue May 17, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
ChristophWurst pushed a commit that referenced this issue May 19, 2020
#2273

Signed-off-by: Cyrille Bollu <cyrpub@bollu.be>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants