DEV: Refactor bookmark modal code (PR #14654)

We had code to open the bookmark modal in two places – the bookmark list and also from within a topic. This caused the two code paths to drift, as in the bookmark list we were not passing in the forTopic or autoDeletePreferences data into the modal, and we were also not refreshing the bookmark list when the bookmark was deleted from within the modal.

This commit moves the modal opening code into an importable function from the controllers/bookmark module, and all callers have to do is pass it an instance of Bookmark and also options for what to do for the following:

  • onAfterSave
  • onAfterDelete
  • onCloseWithoutSaving

GitHub

This feels like a concern for the modal’s controller

I’m not sure I understand the use of a promise here since the promise can either resolve with data and without any data. Also nothing seems to be handling the promise as well.

I have some questions partially due to my lack of understanding of how the modal is implemented but refactoring wise it looks OK to me aside from one comment regarding the modalTitle function.

The promise related stuff is handled in the topic controller:

Just trying to have a lot of nested callbacks.

Thanks @tgxworld you’re right with that title thing I will move it out of the model.