delete message on long press

[fred-bowker]

This is a work in progress pull request, to find the best way to implement delete functionality

[fred-bowker]

delete-functionality-example-min.mov

[fred-bowker]

a different way of handling this is to use a callback in messageMenuAction, however currently that functionality is not used to get callbacks from the standard long press actions, instead it is used to create your own custom menu.

[f3dm76]

I always try to avoid adding more arguments to init - to keep the API as clean as possible, so the user never sees anything he doesn't currently need, so he is not forced to understand extra stuff to simply use the lib out-of-the-box. Moreover this argument will stop doing anything if user chooses not to include delete case to his custom actions menu.

Let's try this: add a modifier
.showDeleteMenuAction(closure: (Message)->())
this modifier will add delete button to standard menu. by default it's not there because it's useless without actual deleting callback, which makes sense I think. so if you want it, just pass your callback here.

Now, this also conflicts with custom menu builder, but if we allow to pass delete closure passed in showDeleteMenuAction into defaultActionClosure, this will be fine, or at least I think so for now, what do you think?

ChatView(messages: viewModel.messages) { draft in
    viewModel.send(draft: draft)
} messageMenuAction: { (action: Action, defaultActionClosure, message) in
    switch action {
    case .reply:
        defaultActionClosure(message, .reply)
    case .edit:
        defaultActionClosure(message, .edit { editedText in
            // update this message's text on your BE
            print(editedText)
        })
     **case .delete:**
           // here user can either use the same approach (showDeleteMenuAction will be called)
           defaultActionClosure(message, .delete)
           // or call the function which is on his end directly 
           // (in this case setting up showDeleteMenuAction is useless, but not harmful. 
           // It is a bit stupid, maybe you'll come up with a better plan as you go)
           viewModel.remove(message)
    }
}
.showDeleteMenuAction { message in
   viewModel.remove(message)
}

[fred-bowker]

The custom menu functionality, adding your own menu options, is there a real use case for it? instead developers could just use the long press menu and making the buttons (options) on that menu customisable. Basically it feels like the code is too complicated for the long press functionality and it would be far simpler if it wasn't including the custom menu functionality, so it would be good to know if anyone is using the custom menu functionality

[f3dm76]

Sorry, I didn't get this part: "instead developers could just use the long press menu and making the buttons (options) on that menu customisable". Isn't it what it currently is? The way I see customizable menu, we need to allow:

• including new options
• excluding default ones
• customizsing name/pics for all of those
• reuse the actual functionality built into some of the default options
• and allow people to use the component which shows the menu on top of chat in the necessary coordinates with scroll if needed, because that's the harderst part of this menu implementation

To allow all of that I introduced the enum, and processor function to reuse the code, and I can't think of a way to make it easier with the same flexibility. If you have ideas on how to simplify it without reducing flexibility, we'll need to make sure the previous API still works or is gently deprecated for the time being

[fred-bowker]

My point more was do you actually need a customisable menu? is this a real world use case, most chat apps are going to need the same functionality Edit, Delete, Copy, Reply etc, why not just include these in an enum and you can have it optional to exclude the ones you aren't using.

Making the menu customisable adds a large amount of complexity to the code, if there is a real world use case whereby people need to add additional items to the menu then sure make it customisable. The use cases that are included in the Example are delete and print. Delete in my view should be included as a core item in the long press menu along with edit.

I can't think of a way to make the code easier with the same flexibility, so my question really is do we need this flexibility as currently i feel like the code complexity for the flexibility is coming at a cost for implementing core longpress functionality such as delete.

[f3dm76]

There could be any number of cases to add there: forward, report, select, pin, etc. Allowing full customization is an important part of this library, so I think it's worth it. I didn't get this part "code complexity for the flexibility is coming at a cost for implementing core longpress". I think the approach I described above is ok good enough to implement delete while not interfering with menu customization mechanism

[fred-bowker]

I need to revisit the code, I think that Edit and Delete should both be core long press functionality and it shouldn't feel like Delete message is a bolt on / special case functionality with any less priority than Edit.

I've changed my mind and agree with you about menu customisation, having the flexibility to add extra items to the long press menu. I'll make the changes to include your suggested approach of:
case .delete:
// here user can either use the same approach (showDeleteMenuAction will be called)
defaultActionClosure(message, .delete)

[fred-bowker]

This follows the standard approach for chat views, "delete for me", "delete for everyone" for messages i have sent, "delete for me" for messages sent to me, tap on background to cancel.

[fred-bowker]

@f3dm76 That is me breaking from the coding for a few days, have a top Christmas and cheers for your reviews over this year and your work on the library.

[f3dm76]

Have a wonderful Christmas, @fred-bowker, thank you for the contributions to the library!