Skip to content

Conversation

@mario
Copy link
Contributor

@mario mario commented Apr 13, 2020

Fixes #2196

Early feedback appreciated.

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

I'm not sure why the tests here are failing so any hint would be welcome. Maybe they're failing in the master as well? Dunno :)

@mario mario requested a review from danxuliu April 14, 2020 08:59
@nickvergessen
Copy link
Member

Can you rebase on latest master? then the icon should be working I guess?

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

Done.

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

Only one thing, I see that the notes conversation is favourite by default, I wouldn't do that..

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

Nice :)

Only one thing, I see that the notes conversation is favourite by default, I wouldn't do that..

Only the first time. If you delete the conversation, once it's re-created it won't be favorite. I copied this part of the experience from some of the leading messengers in the market.

@nickvergessen
Copy link
Member

Only the first time. If you delete the conversation, once it's re-created it won't be favorite.

The thing is a refresh of the room list will automatically readd it, so you can get rid of it for like 29 seconds at best. So we could check the user config instead also for the creation and simply never recreate the conversation instead.

The other thing I would change is using yourself as an author of the sample messages. It has the high advantage, that you can 1. reply to the messages to test things out to begin with and 2. there is less special handling needed in the mobile clients (one less author type to be aware of).

We could also use your own avatar instead? at least the blue = is a bit heavy for me. Would require theming maybe? For the changelog I think its okayish as it is the talk app icon

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

Only the first time. If you delete the conversation, once it's re-created it won't be favorite.

The thing is a refresh of the room list will automatically readd it, so you can get rid of it for like 29 seconds at best. So we could check the user config instead also for the creation and simply never recreate the conversation instead.

Yes, but it will not be favorite anymore. That's what I was pointing at.

The other thing I would change is using yourself as an author of the sample messages. It has the high advantage, that you can 1. reply to the messages to test things out to begin with and 2. there is less special handling needed in the mobile clients (one less author type to be aware of).

I guess so, but this makes more sense to me - an explanation of how to use it. I'm not emotionally attached to the current ways though if there are more people who think we should change it.

We could also use your own avatar instead? at least the blue = is a bit heavy for me. Would require theming maybe? For the changelog I think its okayish as it is the talk app icon

That would be scary :D Anyway, the "blue" should ideally be gradient like our real icon has (and same is true for the changelog icon).

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

As for not recreating it on room refresh: that would mean we need complicated logic in sharing from other apps to always show this conversation since it might not exist in the first place, and a way to create it automagically.

@nickvergessen
Copy link
Member

As for not recreating it on room refresh: that would mean we need complicated logic in sharing from other apps to always show this conversation since it might not exist in the first place, and a way to create it automagically.

Well you deleted it, so it's gone and of course no longer available as a sharing option. If you want it again, create a new conversation and invite no one. ¯_(ツ)_/¯ sounds good enough to me.

@mario
Copy link
Contributor Author

mario commented Apr 14, 2020

As for not recreating it on room refresh: that would mean we need complicated logic in sharing from other apps to always show this conversation since it might not exist in the first place, and a way to create it automagically.

Well you deleted it, so it's gone and of course no longer available as a sharing option. If you want it again, create a new conversation and invite no one. ¯_(ツ)_/¯ sounds good enough to me.

That sounds very, very odd :-/

@nickvergessen
Copy link
Member

That sounds very, very odd :-/

Deleting something and have it popup a split second later with HPB and <30s without is more odd

@marcoambrosini
Copy link
Member

marcoambrosini commented Apr 18, 2020

Deleting something and have it popup a split second later with HPB and <30s without is more odd

Let's not go for the least odd :p

A nice behavior here would be:

  • Provide "My notes" conversation by default;
  • In the actions, provide the option "Disable my notes"
  • In the settings at the bottom of the conversations list, provide a toggle for the "My notes" conversation.
  • If user disables My notes" via the actions, the settings menu should open and a tooltip on the setting should appear, informing the user that the "My notes" conversation can be toggled back from there.

@nickvergessen nickvergessen changed the title Notes 🗒 "My Notes" conversation Apr 20, 2020
@nickvergessen nickvergessen self-assigned this Apr 29, 2020
Signed-off-by: Ivan Sein <[email protected]>
Copy link
Member

@Ivansss Ivansss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API allows to set "My notes" public.
There should be a way to check if the "My notes" is already created show the toggle button displays the correct state.
A warning like "Do you really want to delete "My notes"?" should be shown when unchecking "My notes" optio from settings.

@nickvergessen
Copy link
Member

This is currently so much special code being added duplicating the hack we did for the changelog.
I would propose to do it for 20 properly instead.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! Great work, looking forward to using that. :)

Just one mostly nitpick thing, but important since it seems to be all over in the code:
We never use the "my" form of referring to things (we also try to avoid the "your" and only use it when not avoidable). This is why in the issue it’s specifically called "Note to self". Let’s use that wording.

@Thovi98
Copy link

Thovi98 commented Oct 14, 2020

We never use the "my" form of referring to things (we also try to avoid the "your" and only use it when not avoidable). This is why in the issue it’s specifically called "Note to self". Let’s use that wording.

Hello @jancborchardt ! Just for information, may I ask the reason behind this ? :)

@jancborchardt
Copy link
Member

We never use the "my" form of referring to things (we also try to avoid the "your" and only use it when not avoidable). This is why in the issue it’s specifically called "Note to self". Let’s use that wording.

Hello @jancborchardt ! Just for information, may I ask the reason behind this ? :)

There’s a lot of discussion and writing about using "your *" or "my *" in interfaces, and this is simply our convention. :)

@nickvergessen
Copy link
Member

I will close the PR for now (as there was no active development since 6 months). Anyone can pick it up whenever they want and reopen the PR.

@nickvergessen nickvergessen deleted the notes branch May 2, 2023 20:03
This was referenced Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

📝 Note to self - Overview

7 participants