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

Friendlier room names when calculating from members #1602

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions spec/unit/room.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ describe("Room", function() {
});

room.recalculate();
expect(room.name).toEqual(`${userB} and 2 others`);
expect(room.name).toEqual(`${userB}, ${userC} and ${userD}`);
});

it("missing hero member state reverts to mxid", function() {
Expand All @@ -722,27 +722,30 @@ describe("Room", function() {
});

it("uses counts from summary", function() {
const name = "Mr B";
const firstName = "Bean";
const name = `${firstName} Mr`;
addMember(userB, "join", {name});
room.setSummary({
"m.heroes": [userB],
"m.joined_member_count": 50,
"m.invited_member_count": 50,
});
room.recalculate();
expect(room.name).toEqual(`${name} and 98 others`);
expect(room.name).toEqual(`${firstName} and 98 others`);
});

it("relies on heroes in case of absent counts", function() {
const nameB = "Mr Bean";
const nameC = "Mel C";
const firstNameB = "Bean";
const nameB = `${firstNameB} Mr`;
const firstNameC = "Mel";
const nameC = `${firstNameC} C`;
addMember(userB, "join", {name: nameB});
addMember(userC, "join", {name: nameC});
room.setSummary({
"m.heroes": [userB, userC],
});
room.recalculate();
expect(room.name).toEqual(`${nameB} and ${nameC}`);
expect(room.name).toEqual(`${firstNameB} and ${firstNameC}`);
});

it("uses only heroes", function() {
Expand Down
36 changes: 28 additions & 8 deletions src/models/room.js
Original file line number Diff line number Diff line change
Expand Up @@ -1866,8 +1866,7 @@ function calculateRoomName(room, userId, ignoreRoomNameEvent) {

const joinedMemberCount = room.currentState.getJoinedMemberCount();
const invitedMemberCount = room.currentState.getInvitedMemberCount();
// -1 because these numbers include the syncing user
const inviteJoinCount = joinedMemberCount + invitedMemberCount - 1;
Comment on lines -1869 to -1870
Copy link
Author

Choose a reason for hiding this comment

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

This was actually a bug it seems. inviteJoinCount is only used in the changed if statement below and for passing to memberNamesToRoomName, but memberNamesToRoomName expects the count to be including the current user.

const inviteJoinCount = joinedMemberCount + invitedMemberCount;

// get members that are NOT ourselves and are actually in the room.
let otherNames = null;
Expand All @@ -1890,7 +1889,7 @@ function calculateRoomName(room, userId, ignoreRoomNameEvent) {
otherNames = otherMembers.map((m) => m.name);
}

if (inviteJoinCount) {
if (inviteJoinCount > 1) {
return memberNamesToRoomName(otherNames, inviteJoinCount);
}

Expand Down Expand Up @@ -1926,20 +1925,41 @@ function calculateRoomName(room, userId, ignoreRoomNameEvent) {
}
}

/**
* Simplifies a full name to a single word.
*
* @param {string} name The full name of a room member
* @return {string} A single word of the name
*/
function shortName(name) {
return name.split(" ")[0];
}

function memberNamesToRoomName(names, count = (names.length + 1)) {
const countWithoutMe = count - 1;
if (!names.length) {
return "Empty room";
} else if (names.length === 1 && countWithoutMe <= 1) {
return names[0];
} else if (names.length === 2 && countWithoutMe <= 2) {
return `${names[0]} and ${names[1]}`;
} else {
const plural = countWithoutMe > 1;
return `${shortName(names[0])} and ${shortName(names[1])}`;
} else if (names.length === 3 && countWithoutMe <= 3) {
return `${shortName(names[0])}, ${shortName(names[1])} and ${shortName(names[2])}`;
} else if (names.length >= 2) {
const restCount = countWithoutMe - 2;
const plural = restCount > 1;
if (plural) {
return `${shortName(names[0])}, ${shortName(names[1])} and ${restCount} others`;
} else {
return `${shortName(names[0])}, ${shortName(names[1])} and 1 other`;
}
} else { // Implicit name.length === 1
const restCount = countWithoutMe - 1;
const plural = restCount > 1;
if (plural) {
return `${names[0]} and ${countWithoutMe} others`;
return `${shortName(names[0])} and ${restCount} others`;
} else {
return `${names[0]} and 1 other`;
return `${shortName(names[0])} and 1 other`;
}
}
}
Expand Down