[web] [refactor] remove unneeded div in chat-thread-list component
ClosedPublic
Actions

Authored by • benschac on Mar 10 2022, 1:55 PM.

Details

Reviewers

atul
• jacek
tomek

Commits

rCOMM8de95514f0a0: [web] [refactor] remove unneeded div in chat-thread-list component

Summary

remove an extra div we don't need. I hard-coded a color but will remove it in future diffs, along with other unneeded styles. I just didn't want to break existing functionality.

before:

Image 2022-03-10 at 4.45.37 PM.jpg (1×1 px, 120 KB)

after:

Image 2022-03-10 at 4.45.12 PM.jpg (1×1 px, 225 KB)

Test Plan

do a side-by-side with production. Make sure the unread pop up doesn't break.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• benschac created this revision.Mar 10 2022, 1:55 PM

Herald added subscribers: • karol, • adrian, tomek, ashoat. · View Herald TranscriptMar 10 2022, 1:55 PM

• benschac edited the summary of this revision. (Show Details)Mar 10 2022, 1:56 PM

• benschac added reviewers: • jacek, tomek.

• benschac edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B7292: Diff 10282.Mar 10 2022, 1:59 PM

• benschac requested review of this revision.Mar 10 2022, 1:59 PM

I hard-coded a color but will remove it in future diffs, along with other unneeded styles. I just didn't want to break existing functionality.

If removing the div requires adding these colors, then adding them was ok, but I think we should still use variables instead of hardcoded values.

This revision is now accepted and ready to land.Mar 11 2022, 5:44 AM

In D3405#92013, @palys-swm wrote:

I hard-coded a color but will remove it in future diffs, along with other unneeded styles. I just didn't want to break existing functionality.

If removing the div requires adding these colors, then adding them was ok, but I think we should still use variables instead of hardcoded values.

Agreed. I'm going to use variables in the future. Just wanted to get rid of this div.

Closed by commit rCOMM8de95514f0a0: [web] [refactor] remove unneeded div in chat-thread-list component (authored by • benschac). · Explain WhyMar 11 2022, 7:22 AM

This revision was automatically updated to reflect the committed changes.

• benschac added a commit: rCOMM8de95514f0a0: [web] [refactor] remove unneeded div in chat-thread-list component.

• benschac added a reverting change: D3407: Revert "[web] [refactor] remove unneeded div in chat-thread-list component".Mar 11 2022, 10:35 AM

• benschac added a reverting change: rCOMM7d8e81f8dddf: Revert "[web] [refactor] remove unneeded div in chat-thread-list component".Mar 11 2022, 12:48 PM

Revision Contents
Changeset List

Path

Size

web/

chat/

chat-thread-list-item-menu.react.js

8 lines

chat-thread-list.css

2 lines

Diff 10299

View Options

web/chat/chat-thread-list-item-menu.react.js

Show First 20 Lines • Show All 368 Lines • ▼ Show 20 Lines	currentUser: {
subscription: {		subscription: {
pushNotifs: false,		pushNotifs: false,
home: false,		home: false,
},		},
unread: false,		unread: false,
},		},
repliesCount: 0,		repliesCount: 0,
sourceMessageID,		sourceMessageID,
		pinnedCount: 0,
};		};

const userInfos = {};		const userInfos = {};
for (const member of members) {		for (const member of members) {
const { id, username } = member;		const { id, username } = member;
userInfos[id] = { id, username };		userInfos[id] = { id, username };
}		}

▲ Show 20 Lines • Show All 307 Lines • ▼ Show 20 Lines
type RawThreadInfoOptions = {		type RawThreadInfoOptions = {
+includeVisibilityRules?: ?boolean,		+includeVisibilityRules?: ?boolean,
+filterMemberList?: ?boolean,		+filterMemberList?: ?boolean,
+hideThreadStructure?: ?boolean,		+hideThreadStructure?: ?boolean,
+shimThreadTypes?: ?{		+shimThreadTypes?: ?{
+[inType: ThreadType]: ThreadType,		+[inType: ThreadType]: ThreadType,
},		},
+filterDetailedThreadEditPermissions?: boolean,		+filterDetailedThreadEditPermissions?: boolean,
		+excludePinnedCount?: boolean,
};		};
function rawThreadInfoFromServerThreadInfo(		function rawThreadInfoFromServerThreadInfo(
serverThreadInfo: ServerThreadInfo,		serverThreadInfo: ServerThreadInfo,
viewerID: string,		viewerID: string,
options?: RawThreadInfoOptions,		options?: RawThreadInfoOptions,
): ?RawThreadInfo {		): ?RawThreadInfo {
const includeVisibilityRules = options?.includeVisibilityRules;		const includeVisibilityRules = options?.includeVisibilityRules;
const filterMemberList = options?.filterMemberList;		const filterMemberList = options?.filterMemberList;
const hideThreadStructure = options?.hideThreadStructure;		const hideThreadStructure = options?.hideThreadStructure;
const shimThreadTypes = options?.shimThreadTypes;		const shimThreadTypes = options?.shimThreadTypes;
const filterDetailedThreadEditPermissions =		const filterDetailedThreadEditPermissions =
options?.filterDetailedThreadEditPermissions;		options?.filterDetailedThreadEditPermissions;
		const excludePinnedCount = options?.excludePinnedCount;

const members = [];		const members = [];
let currentUser;		let currentUser;
for (const serverMember of serverThreadInfo.members) {		for (const serverMember of serverThreadInfo.members) {
if (		if (
filterMemberList &&		filterMemberList &&
serverMember.id !== viewerID &&		serverMember.id !== viewerID &&
!serverMember.role &&		!serverMember.role &&
▲ Show 20 Lines • Show All 87 Lines • ▼ Show 20 Lines	if (serverThreadInfo.avatar) {
rawThreadInfo = { ...rawThreadInfo, avatar: serverThreadInfo.avatar };		rawThreadInfo = { ...rawThreadInfo, avatar: serverThreadInfo.avatar };
}		}
if (includeVisibilityRules) {		if (includeVisibilityRules) {
return {		return {
...rawThreadInfo,		...rawThreadInfo,
visibilityRules: rawThreadInfo.type,		visibilityRules: rawThreadInfo.type,
};		};
}		}
		if (!excludePinnedCount) {
		return {
		...rawThreadInfo,
		pinnedCount: serverThreadInfo.pinnedCount,
		};
		}
return rawThreadInfo;		return rawThreadInfo;
}		}

function filterThreadEditDetailedPermissions(		function filterThreadEditDetailedPermissions(
permissions: ThreadPermissionsInfo,		permissions: ThreadPermissionsInfo,
shouldFilter: ?boolean,		shouldFilter: ?boolean,
): ThreadPermissionsInfo {		): ThreadPermissionsInfo {
if (!shouldFilter) {		if (!shouldFilter) {
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Lines	let threadInfo: ThreadInfo = {
roles: rawThreadInfo.roles,		roles: rawThreadInfo.roles,
currentUser: getCurrentUser(rawThreadInfo, viewerID, userInfos),		currentUser: getCurrentUser(rawThreadInfo, viewerID, userInfos),
repliesCount: rawThreadInfo.repliesCount,		repliesCount: rawThreadInfo.repliesCount,
};		};
threadInfo = {		threadInfo = {
...threadInfo,		...threadInfo,
uiName: threadUIName(threadInfo),		uiName: threadUIName(threadInfo),
};		};
const { sourceMessageID, avatar } = rawThreadInfo;		const { sourceMessageID, avatar, pinnedCount } = rawThreadInfo;
if (sourceMessageID) {		if (sourceMessageID) {
threadInfo = { ...threadInfo, sourceMessageID };		threadInfo = { ...threadInfo, sourceMessageID };
}		}

if (avatar) {		if (avatar) {
threadInfo = { ...threadInfo, avatar };		threadInfo = { ...threadInfo, avatar };
} else if (		} else if (
rawThreadInfo.type === threadTypes.PERSONAL \|\|		rawThreadInfo.type === threadTypes.PERSONAL \|\|
rawThreadInfo.type === threadTypes.PRIVATE		rawThreadInfo.type === threadTypes.PRIVATE
) {		) {
threadInfo = {		threadInfo = {
...threadInfo,		...threadInfo,
avatar: getUserAvatarForThread(rawThreadInfo, viewerID, userInfos),		avatar: getUserAvatarForThread(rawThreadInfo, viewerID, userInfos),
};		};
}		}

		if (pinnedCount) {
		threadInfo = { ...threadInfo, pinnedCount };
		}

return threadInfo;		return threadInfo;
}		}

function getCurrentUser(		function getCurrentUser(
threadInfo: RawThreadInfo \| ThreadInfo,		threadInfo: RawThreadInfo \| ThreadInfo,
viewerID: ?string,		viewerID: ?string,
userInfos: UserInfos,		userInfos: UserInfos,
): ThreadCurrentUserInfo {		): ThreadCurrentUserInfo {
▲ Show 20 Lines • Show All 712 Lines • Show Last 20 Lines