Fix Keep message drafts #1459 #1891
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1891 +/- ##
==========================================
- Coverage 60.54% 60.44% -0.11%
==========================================
Files 269 269
Lines 5566 5577 +11
Branches 901 903 +2
==========================================
+ Hits 3370 3371 +1
- Misses 2195 2205 +10
Partials 1 1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks, that should work! I left some comments.
Another option that seems a bit more in line with the Karrot architecture would to sync message drafts to the conversations vuex datastore and use vuex-persistedstate to sync it to localStorage. We already have a 'persistedState' config that could be extended: https://github.com//yunity/karrot-frontend/blob/master/src/base/datastore/persistedState.js
Would you be interested in implementing this approach?
| if (this.key) { | ||
| return this.key | ||
| } | ||
| else return 'wallConv' |
There was a problem hiding this comment.
This doesn't need to be a special case, wall conversations have an ID as well. You can pass it from WallConversation.
There was a problem hiding this comment.
in fact, i could get conversationId from editing an existing message but for the one on the top which enables to create a new message, we need an identifier. but what I did was seting the key in the parent component so in my new version it is consistent to just return the key.
| }, | ||
| onFocus (event) { | ||
| this.hasFocus = true | ||
| }, | ||
| onBlur () { | ||
| this.hasFocus = false | ||
| this.message = '' |
There was a problem hiding this comment.
Seems unexpected to delete the message on blur!
| @@ -93,6 +93,9 @@ export default { | |||
| type: String, | |||
| default: null, | |||
| }, | |||
| conversation: { | |||
There was a problem hiding this comment.
I think the prop should be named conversationId, to avoid confusion with a complete conversation object.
| hasFocus: false, | ||
| key: this.conversation, |
There was a problem hiding this comment.
This is not reactive: if conversation ID gets loaded lazily (i.e. after this component initializes) or changes, key will be invalid.
There was a problem hiding this comment.
agreed I switched that to a computed prop
| }, | ||
| leaveEdit () { | ||
| this.$emit('leaveEdit') | ||
| this.message = '' | ||
| localStorage.removeItem(this.getkey()) |
There was a problem hiding this comment.
There's a small bug in this: it would delete a message draft from localStorage when editing a previous message in the same conversation.
For example, I start writing a new message to you, then edit an old message in our conversation but abort editing and close my browser. I would expect the new message to still exist when I open the browser again.
There was a problem hiding this comment.
on the wall there is a cancel button that user can use to cancel the editing and then accept the draft to be deleted but on pickup messages the only way to cancel is to press the escape button. I think it would make sense to add a button here and in such case, I agree this deletion is not necessary. but without this button, i guess most users will just leave the draft thinking that it would cancel the draft. It is more a UX question
There was a problem hiding this comment.
I'm not sure what you are referring to: as far as I know, the "cancel edit" button shown in both WallConversation and ChatConversation.
|
HI @tiltec I just pushed a new commit that addresses most of the points mentioned. I unfied the way we handle the conversationId, even for wall conversation. I kept a 'wallConv, key to keep the draft of new wall conversation( no iD as long as message is not submitted). I kept the deletion of the draft on Leave, because it is the only way to delete the draft. As explained earlier it is a UI decision, you can remove if your prefer, but that would involve the localstorage will never get cleaned. I will want on an alterbative leveraging vuex and vuex-persistant as you mentioned, but I not sure it is a good idea as the data is really used only in a single component (ConversationCompose). I wanted to finalise his PR so at leats we have something that works. as of now people can also edit existing messages. the message showing on the page is the one that is coming from backend, but if they come back to finalise an edit they started earlier, this draft will be used in the edit form. it is only if they submit that the draft will replace previous message. Please test it and let me know if it does the job. |
There's more than one wall: each group and each place has one. You implemented it in a way that would remember one message draft for walls, no matter where the user originally typed it in. |
|
@tiltec Thanks for your feedback, In fact I am using conversation in all the places. as of today, I saw usage of ConversationCompose on the wall and in pickups. for both of them I am using the conversationId so it is universal and it wil store all drafts based on this conversationId. but if you look at the top of the wall page, users can enter directly in Converation compose to edit a new conversation. This conversation can not have an idea because it is not yet part of any conversation. this is only for this one that i created the wallConv. if people edit an existing conversation or add new comment on an existing conversation, the feature will save the draft based on its conversationId. because local storage is used, the draft will be saved only for this specific user (and specific device in fact) so there is no conflict with other groups or places. But if it happens that users are part of different groups or places then I would have to use the groupId and PlaceId in my key used in local storage. that would be easy to implement |
|
This pull request is marked as stale because it has not had any activity for 90 days, remove the stale label or add a comment on it, otherwise it will be automatically closed in 7 days. Thanks! |
Closes ___ (e.g. #230)
What does this PR do?
use localStorage to keep draft of messages (wall and pickups messages) even if page is refreshed. The Key refers to the ConversationId and the value to the messsage. The key-value pair is destroyed only when the draft is submited, or the user voluntary quit the page or escape the form.
(briefly describe what this PR is about)
Links to related issues
[https://github.com//issues/1459]
)## Checklist