Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calls are Conversations #919

Merged
merged 1 commit into from
May 30, 2018
Merged

Calls are Conversations #919

merged 1 commit into from
May 30, 2018

Conversation

Ivansss
Copy link
Member

@Ivansss Ivansss commented May 29, 2018

Follow up #751

Activities:

{User} invited you to a call (before)
{User} invited you to a conversation (now)

Notifications:

{User} invited you to a group call (before)
{User} invited you to a group conversation (now)

{User} mentioned you in a group chat (before)
{User} mentioned you in a group conversation (now)

Signed-off-by: Ivan Sein <ivan@nextcloud.com>
@Ivansss Ivansss added this to the 3.3 (Nextcloud 13.0.3/4) milestone May 29, 2018
@@ -126,7 +126,7 @@ protected function getRoom(IL10N $l, Room $room) {
return [
'type' => 'call',
'id' => $room->getId(),
'name' => $room->getName() ?: $l->t('a call'),
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably remain "a call" since it's of type call?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually might be right, since notifier users different stuff for sending a call.

@@ -140,7 +140,7 @@ protected function getFormerRoom(IL10N $l, $roomId) {
return [
'type' => 'call',
'id' => $roomId,
'name' => $l->t('a call'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@mario
Copy link
Contributor

mario commented May 29, 2018

Please check.

@mario
Copy link
Contributor

mario commented May 29, 2018

👍

@mario
Copy link
Contributor

mario commented May 29, 2018

Looks good by me.

@Ivansss Ivansss merged commit 323d64e into master May 30, 2018
@Ivansss Ivansss deleted the calls-are-conversations branch May 30, 2018 07:48
@jancborchardt
Copy link
Member

jancborchardt commented May 30, 2018

If it’s a group conversation, we can directly show the name. So instead of:

Ivan mentioned you in a group conversation: Spreed hackers

it could be:

Ivan mentioned you in Spreed hackers

Because this is much quicker to read and also works better on mobile, especially for long user names. (And yeah, is the bolding possible?)

@danxuliu
Copy link
Member

is the bolding possible?

Format is not sent nor enforced by the server (and it should not be); clients just receive {user} mentioned you in a group conversation: {conversation}, and it is up to them how to format the parameters.

For example, the notifications app in the WebUI uses bold text for the conversation and user names, and it also shows the user avatar.

@jancborchardt
Copy link
Member

@danxuliu ok, then ignore the bolding but let’s please shorten the text. :)

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.

4 participants