Single message drafts #772

kaustubh-nair · 2020-08-12T20:59:19Z

This PR adds support for a minimal draft message feature, storing a single
draft message when meta s is pressed.

neiljp

@kaustubh-nair While this is limited to one draft and one session (ie. isn't saved), this promises to be useful in terms of keeping one message 'available' while browsing around others 👍

There is the overlap commit with #746 to consider, but this seems fairly straightforward given the limited scope (not saving during/between sessions or on crashing, no server interaction)

There is currently the minor 'surprise' of a second draft overwriting another with no notice, ie. one just containing one letter (or a space) will overwrite the previous. Perhaps have a yes/no popup to check with the user?

zulipterminal/model.py

README.md

zulipterminal/config/keys.py

zulipterminal/ui.py

neiljp · 2020-08-13T22:05:13Z

@kaustubh-nair I just pushed the first commit as c7026b3 with a reworded general commit text. I'm marking this as not needing review after our discussion on czo, but let me know if you want more details or discussion.

This should allow @preetmishra to implement #708 more simply too.

neiljp

@kaustubh-nair This looks good 👍 See my comments on czo re commit text and key choice - these comments are more code-related after reading in more detail.

neiljp · 2020-08-23T02:15:19Z

zulipterminal/model.py

@@ -285,6 +286,10 @@ def react_to_message(self,
            response = self.client.add_reaction(reaction_to_toggle_spec)
        display_error_if_present(response, self.controller)

+    def save_draft(self, message: Message) -> None:
+        self.draft = message
+        self.controller.view.set_footer_text("Saved message as draft", 3)


Should this line go in the core popup method?

@neiljp You mean in PopUpConfirmationView?

In the function that is called when 'yes' is selected.

model.save_draft() is the method that is called when 'yes' is selected? I'm confused 🤔

save_draft is the method that's called currently, but I was wondering if we should call the set_footer_text from the popup too. We already have a lot of ui-coupled code in the model, which I'd like to move away from, but let's discuss and that can be something we do later.

zulipterminal/ui.py

zulipterminal/ui_tools/boxes.py

zulipterminal/ui_tools/views.py

This commit removes the return statement since it is not being used anywhere. Updated the type of callback function passed in PopUpConfirmationView appropriately.

kaustubh-nair · 2020-08-23T16:42:16Z

@neiljp Thanks for the review, PR updated!

neiljp

@kaustubh-nair The changes from czo look OK 👍 However, there are a number of aspects that seem to have been missed from the last review and there are a few other points that I think we need to address before merging.

tests/ui_tools/test_boxes.py

zulipterminal/config/keys.py

neiljp · 2020-08-24T02:05:20Z

zulipterminal/config/keys.py

+    ('OPEN_DRAFT', {
+        'keys': {'d'},
+        'help_text': 'Open draft message saved in this session',
+        'key_category': 'general',
+    }),


I'd suggest we want this below the about menu entry?

zulipterminal/ui_tools/boxes.py

neiljp · 2020-08-24T02:09:22Z

zulipterminal/model.py

@@ -285,6 +286,10 @@ def react_to_message(self,
            response = self.client.add_reaction(reaction_to_toggle_spec)
        display_error_if_present(response, self.controller)

+    def save_draft(self, message: Message) -> None:
+        self.draft = message
+        self.controller.view.set_footer_text("Saved message as draft", 3)


In the function that is called when 'yes' is selected.

zulipterminal/model.py

This adds support for a minimal draft message feature, storing a single draft message. The draft is stored for each session, and server-side storage will be added later when the API endpoints are available. It functions as follows: * On pressing SAVE_AS_DRAFT(`meta s`), a popup is shown confirming whether the message should be saved as a draft. This is not permitted when a message is being edited, or the current message is the same as a saved draft. The pop-up is bypassed when there is no saved draft. * The saved draft can be opened for editing by using the OPEN_DRAFT(`d`) key.

kaustubh-nair · 2020-08-24T13:37:22Z

@neiljp Thanks for the review, PR updated.

neiljp · 2020-08-27T06:49:31Z

@kaustubh-nair This will be useful, thanks for working on this! 👍 I just merged this with a slight adjustment to the README to match the order of the hotkeys in the help menu 🎉

zulipbot added the size: M [Automatic label added by zulipbot] label Aug 12, 2020

kaustubh-nair force-pushed the drafts_v1 branch from 7da16d3 to 7db0d39 Compare August 12, 2020 21:00

kaustubh-nair added the PR needs review PR requires feedback to proceed label Aug 12, 2020

neiljp mentioned this pull request Aug 13, 2020

Recipient box typeahead for streams and topics #746

Closed

neiljp reviewed Aug 13, 2020

View reviewed changes

zulipterminal/model.py Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

zulipterminal/config/keys.py Outdated Show resolved Hide resolved

zulipterminal/ui.py Outdated Show resolved Hide resolved

neiljp removed the PR needs review PR requires feedback to proceed label Aug 13, 2020

kaustubh-nair force-pushed the drafts_v1 branch from 7db0d39 to dd648e5 Compare August 13, 2020 16:06

zulipbot added size: L [Automatic label added by zulipbot] and removed size: M [Automatic label added by zulipbot] labels Aug 13, 2020

kaustubh-nair force-pushed the drafts_v1 branch 2 times, most recently from f172d25 to 67266a8 Compare August 13, 2020 17:43

kaustubh-nair added the PR needs review PR requires feedback to proceed label Aug 13, 2020

neiljp removed the PR needs review PR requires feedback to proceed label Aug 13, 2020

neiljp added this to the Next Release milestone Aug 16, 2020

kaustubh-nair force-pushed the drafts_v1 branch 3 times, most recently from 8b5f166 to ac9e187 Compare August 22, 2020 18:51

kaustubh-nair added the PR needs review PR requires feedback to proceed label Aug 22, 2020

kaustubh-nair force-pushed the drafts_v1 branch from ac9e187 to 5f9d138 Compare August 22, 2020 18:54

neiljp reviewed Aug 23, 2020

View reviewed changes

neiljp removed the PR needs review PR requires feedback to proceed label Aug 23, 2020

model/views: Remove return statement from toggle_stream_muted_status.

57716eb

This commit removes the return statement since it is not being used anywhere. Updated the type of callback function passed in PopUpConfirmationView appropriately.

kaustubh-nair force-pushed the drafts_v1 branch 2 times, most recently from 99698ae to ce55965 Compare August 23, 2020 16:37

kaustubh-nair added feedback wanted PR needs review PR requires feedback to proceed labels Aug 23, 2020

kaustubh-nair force-pushed the drafts_v1 branch from ce55965 to 72a1c46 Compare August 23, 2020 16:59

neiljp reviewed Aug 24, 2020

View reviewed changes

neiljp removed the PR needs review PR requires feedback to proceed label Aug 24, 2020

kaustubh-nair force-pushed the drafts_v1 branch 2 times, most recently from 074a36e to 4e144e5 Compare August 24, 2020 09:03

kaustubh-nair force-pushed the drafts_v1 branch from 4e144e5 to 22d2e5b Compare August 24, 2020 09:04

neiljp closed this Aug 27, 2020

neiljp mentioned this pull request Oct 22, 2020

TRACKING: Develop drafts feature #302

Open

Uh oh!

Single message drafts #772

Single message drafts #772

Uh oh!

Conversation

kaustubh-nair commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neiljp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

neiljp commented Aug 13, 2020

Uh oh!

neiljp left a comment

Choose a reason for hiding this comment

Uh oh!

neiljp Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

kaustubh-nair Aug 23, 2020

Choose a reason for hiding this comment

Uh oh!

neiljp Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

kaustubh-nair Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

neiljp Aug 27, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kaustubh-nair commented Aug 23, 2020

Uh oh!

neiljp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

neiljp Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

neiljp Aug 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kaustubh-nair commented Aug 24, 2020

Uh oh!

neiljp commented Aug 27, 2020

Uh oh!

Uh oh!

kaustubh-nair commented Aug 12, 2020 •

edited

Loading