Page MenuHomePhabricator
Differential D4577

[lib, native, web] Remove showingSidebarsInline variable and button text option "See threads..."
ClosedPublic
Actions

Authored by inka on Jul 19 2022, 7:01 AM.
Tags
None
Referenced Files
F6236606: D4577.diff
Thu, Apr 24, 3:37 PM
Unknown Object (File)
Wed, Apr 23, 5:23 PM
Unknown Object (File)
Wed, Apr 23, 5:23 PM
Unknown Object (File)
Mon, Apr 21, 1:14 PM
Unknown Object (File)
Mon, Apr 21, 1:13 PM
Unknown Object (File)
Mon, Apr 21, 1:13 PM
Unknown Object (File)
Tue, Apr 8, 8:02 PM
Unknown Object (File)
Tue, Apr 8, 8:02 PM
View All Files
Subscribers
• abosh
• adrian
atul

Details

Reviewers
ashoat
tomek
Commits
rCOMMe7979bf66618: [lib, native, web] Remove showingSidebarsInline variable and button text option…
Summary

There was some obsolete code that got never called since diff D1487. It got removed.
showingSidebarsInline was indeed obsolete as the only function that used it - ChatThreadListSeeMoreSidebars - was called inside an ‘if’ not allowing sidebarsItems.length to be 0 and the variable was being set as showingSidebarsInline: sidebarItems.length !== 0
Thus the code defined for when showingSidebarsInline == 0 got removed as well.

Linear issue: https://linear.app/comm/issue/ENG-1426/delete-see-threads-button-text-option

Test Plan

Check that everything still compiles.

Diff Detail

Repository
rCOMM Comm
Branch
inka/delete_see_threads_text
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Jul 19 2022, 7:01 AM
Herald added subscribers: • abosh, atul, • adrian. · View Herald TranscriptJul 19 2022, 7:02 AM
Harbormaster completed remote builds in B10646: Diff 14646.Jul 19 2022, 7:14 AM
inka requested review of this revision.Jul 19 2022, 7:14 AM
inka retitled this revision from Remove showingSidebarsInline variable and button text option "See threads..." to [lib, native, web] Remove showingSidebarsInline variable and button text option "See threads...".Jul 19 2022, 7:20 AM
ashoat accepted this revision.Jul 19 2022, 8:47 AM

Thank you!!

native/chat/chat-thread-list-see-more-sidebars.react.js
28 ↗(On Diff #14646)

We can probably just move this to appear inline on line 38 now

web/chat/chat-thread-list-see-more-sidebars.react.js
24 ↗(On Diff #14646)

We can probably just move this to appear inline on line 36 now

This revision is now accepted and ready to land.Jul 19 2022, 8:47 AM
inka updated this revision to Diff 14648.Jul 19 2022, 9:23 AM

Inline variable buttonText in chat-thread-list-see-more-sidebars.react.js

inka edited the summary of this revision. (Show Details)Jul 19 2022, 9:34 AM
Harbormaster completed remote builds in B10648: Diff 14648.Jul 19 2022, 9:36 AM
Closed by commit rCOMMe7979bf66618: [lib, native, web] Remove showingSidebarsInline variable and button text option… (authored by • InkaSokolowska). · Explain WhyJul 20 2022, 1:09 AM
This revision was automatically updated to reflect the committed changes.
• InkaSokolowska added a commit: rCOMMe7979bf66618: [lib, native, web] Remove showingSidebarsInline variable and button text option….

Revision Contents
Changeset List

Diff 14648

View Options

lib/selectors/chat-selectors.js

Show First 20 Lines • Show All 311 Lines • ▼ Show 20 Lines (navigationState: ?PossiblyStaleNavigationState) => {
// Finally, we want to disable the swipe gesture if there is a stack with // Finally, we want to disable the swipe gesture if there is a stack with
// more than one subroute, since then the stack will have its own swipe // more than one subroute, since then the stack will have its own swipe
// gesture that will conflict with the drawer's // gesture that will conflict with the drawer's
return currentTabSubrouteState.routes.length < 2; return currentTabSubrouteState.routes.length < 2;
}, },
); );
const getChatNavState: (
navigationState: ?PossiblyStaleNavigationState,
) => ?PossiblyStaleNavigationState = (
navigationState: ?PossiblyStaleNavigationState,
) => {
tomekUnsubmitted
Done
Inline Actions
},
);
- const getChatNavState: (
- navigationState: ?PossiblyStaleNavigationState,
- ) => ?PossiblyStaleNavigationState = (
- navigationState: ?PossiblyStaleNavigationState,
- ) => {
+ function getChatNavState(navigationState: ?PossiblyStaleNavigationState): ?PossiblyStaleNavigationState {
if (!navigationState) {

It might be more readable using function syntax

tomek: It might be more readable using `function` syntax
if (!navigationState) {
return null;
}
const [firstAppSubroute] = navigationState.routes;
if (firstAppSubroute.name !== CommunityDrawerNavigatorRouteName) {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Usually the first route is the AppRouteName. Why is this different?

inka: Usually the first route is the `AppRouteName`. Why is this different?
kubaAuthorUnsubmitted
Done
Inline Actions

It is called from`OverlayRouter`, so it is nested.

kuba: It is called from`OverlayRouter`, so it is nested.
const communityDrawerState = getStateFromNavigatorRoute(firstAppSubroute);
const [firstCommunityDrawerSubroute] = communityDrawerState.routes;
if (firstCommunityDrawerSubroute.name !== TabNavigatorRouteName) {
return null;
}
const tabState = getStateFromNavigatorRoute(firstCommunityDrawerSubroute);
if (!tabState) {
return null;
}
let chatRoute;
for (const route of tabState.routes) {
if (route.name === ChatRouteName) {
chatRoute = route;
break;
}
}
if (!chatRoute || !chatRoute.state) {
return null;
}
const chatRouteState = getStateFromNavigatorRoute(chatRoute);
if (chatRouteState.type !== 'stack') {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Why is this necessary? When can chatRouteState.type not be 'stack'? Or is this something flow requires?

inka: Why is this necessary? When can `chatRouteState.type` not be `'stack'`? Or is this something…
kubaAuthorUnsubmitted
Done
Inline Actions

This function is based on drawerSwipeEnabledSelector function. It might be unnecessary. Not sure if I should remove it.

kuba: This function is based on `drawerSwipeEnabledSelector` function. It might be unnecessary. Not…
return chatRouteState;
};
const getRemoveEditMode: ( const getRemoveEditMode: (
chatRouteState: ?PossiblyStaleNavigationState, chatRouteState: ?PossiblyStaleNavigationState,
) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => { ) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => {
if (!chatRouteState) { if (!chatRouteState) {
return null; return null;
} }
const messageListRoute = const messageListRoute =
chatRouteState.routes[chatRouteState.routes.length - 1]; chatRouteState.routes[chatRouteState.routes.length - 1];
Show All 29 Linesexport {
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getChatNavState,
}; };
View Options

native/chat/chat-thread-list-item.react.js

Show First 20 Lines • Show All 311 Lines • ▼ Show 20 Lines (navigationState: ?PossiblyStaleNavigationState) => {
// Finally, we want to disable the swipe gesture if there is a stack with // Finally, we want to disable the swipe gesture if there is a stack with
// more than one subroute, since then the stack will have its own swipe // more than one subroute, since then the stack will have its own swipe
// gesture that will conflict with the drawer's // gesture that will conflict with the drawer's
return currentTabSubrouteState.routes.length < 2; return currentTabSubrouteState.routes.length < 2;
}, },
); );
const getChatNavState: (
navigationState: ?PossiblyStaleNavigationState,
) => ?PossiblyStaleNavigationState = (
navigationState: ?PossiblyStaleNavigationState,
) => {
tomekUnsubmitted
Done
Inline Actions
},
);
- const getChatNavState: (
- navigationState: ?PossiblyStaleNavigationState,
- ) => ?PossiblyStaleNavigationState = (
- navigationState: ?PossiblyStaleNavigationState,
- ) => {
+ function getChatNavState(navigationState: ?PossiblyStaleNavigationState): ?PossiblyStaleNavigationState {
if (!navigationState) {

It might be more readable using function syntax

tomek: It might be more readable using `function` syntax
if (!navigationState) {
return null;
}
const [firstAppSubroute] = navigationState.routes;
if (firstAppSubroute.name !== CommunityDrawerNavigatorRouteName) {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Usually the first route is the AppRouteName. Why is this different?

inka: Usually the first route is the `AppRouteName`. Why is this different?
kubaAuthorUnsubmitted
Done
Inline Actions

It is called from`OverlayRouter`, so it is nested.

kuba: It is called from`OverlayRouter`, so it is nested.
const communityDrawerState = getStateFromNavigatorRoute(firstAppSubroute);
const [firstCommunityDrawerSubroute] = communityDrawerState.routes;
if (firstCommunityDrawerSubroute.name !== TabNavigatorRouteName) {
return null;
}
const tabState = getStateFromNavigatorRoute(firstCommunityDrawerSubroute);
if (!tabState) {
return null;
}
let chatRoute;
for (const route of tabState.routes) {
if (route.name === ChatRouteName) {
chatRoute = route;
break;
}
}
if (!chatRoute || !chatRoute.state) {
return null;
}
const chatRouteState = getStateFromNavigatorRoute(chatRoute);
if (chatRouteState.type !== 'stack') {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Why is this necessary? When can chatRouteState.type not be 'stack'? Or is this something flow requires?

inka: Why is this necessary? When can `chatRouteState.type` not be `'stack'`? Or is this something…
kubaAuthorUnsubmitted
Done
Inline Actions

This function is based on drawerSwipeEnabledSelector function. It might be unnecessary. Not sure if I should remove it.

kuba: This function is based on `drawerSwipeEnabledSelector` function. It might be unnecessary. Not…
return chatRouteState;
};
const getRemoveEditMode: ( const getRemoveEditMode: (
chatRouteState: ?PossiblyStaleNavigationState, chatRouteState: ?PossiblyStaleNavigationState,
) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => { ) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => {
if (!chatRouteState) { if (!chatRouteState) {
return null; return null;
} }
const messageListRoute = const messageListRoute =
chatRouteState.routes[chatRouteState.routes.length - 1]; chatRouteState.routes[chatRouteState.routes.length - 1];
Show All 29 Linesexport {
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getChatNavState,
}; };
View Options

native/chat/chat-thread-list-see-more-sidebars.react.js

Show First 20 Lines • Show All 311 Lines • ▼ Show 20 Lines (navigationState: ?PossiblyStaleNavigationState) => {
// Finally, we want to disable the swipe gesture if there is a stack with // Finally, we want to disable the swipe gesture if there is a stack with
// more than one subroute, since then the stack will have its own swipe // more than one subroute, since then the stack will have its own swipe
// gesture that will conflict with the drawer's // gesture that will conflict with the drawer's
return currentTabSubrouteState.routes.length < 2; return currentTabSubrouteState.routes.length < 2;
}, },
); );
const getChatNavState: (
navigationState: ?PossiblyStaleNavigationState,
) => ?PossiblyStaleNavigationState = (
navigationState: ?PossiblyStaleNavigationState,
) => {
tomekUnsubmitted
Done
Inline Actions
},
);
- const getChatNavState: (
- navigationState: ?PossiblyStaleNavigationState,
- ) => ?PossiblyStaleNavigationState = (
- navigationState: ?PossiblyStaleNavigationState,
- ) => {
+ function getChatNavState(navigationState: ?PossiblyStaleNavigationState): ?PossiblyStaleNavigationState {
if (!navigationState) {

It might be more readable using function syntax

tomek: It might be more readable using `function` syntax
if (!navigationState) {
return null;
}
const [firstAppSubroute] = navigationState.routes;
if (firstAppSubroute.name !== CommunityDrawerNavigatorRouteName) {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Usually the first route is the AppRouteName. Why is this different?

inka: Usually the first route is the `AppRouteName`. Why is this different?
kubaAuthorUnsubmitted
Done
Inline Actions

It is called from`OverlayRouter`, so it is nested.

kuba: It is called from`OverlayRouter`, so it is nested.
const communityDrawerState = getStateFromNavigatorRoute(firstAppSubroute);
const [firstCommunityDrawerSubroute] = communityDrawerState.routes;
if (firstCommunityDrawerSubroute.name !== TabNavigatorRouteName) {
return null;
}
const tabState = getStateFromNavigatorRoute(firstCommunityDrawerSubroute);
if (!tabState) {
return null;
}
let chatRoute;
for (const route of tabState.routes) {
if (route.name === ChatRouteName) {
chatRoute = route;
break;
}
}
if (!chatRoute || !chatRoute.state) {
return null;
}
const chatRouteState = getStateFromNavigatorRoute(chatRoute);
if (chatRouteState.type !== 'stack') {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Why is this necessary? When can chatRouteState.type not be 'stack'? Or is this something flow requires?

inka: Why is this necessary? When can `chatRouteState.type` not be `'stack'`? Or is this something…
kubaAuthorUnsubmitted
Done
Inline Actions

This function is based on drawerSwipeEnabledSelector function. It might be unnecessary. Not sure if I should remove it.

kuba: This function is based on `drawerSwipeEnabledSelector` function. It might be unnecessary. Not…
return chatRouteState;
};
const getRemoveEditMode: ( const getRemoveEditMode: (
chatRouteState: ?PossiblyStaleNavigationState, chatRouteState: ?PossiblyStaleNavigationState,
) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => { ) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => {
if (!chatRouteState) { if (!chatRouteState) {
return null; return null;
} }
const messageListRoute = const messageListRoute =
chatRouteState.routes[chatRouteState.routes.length - 1]; chatRouteState.routes[chatRouteState.routes.length - 1];
Show All 29 Linesexport {
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getChatNavState,
}; };
View Options

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

Show First 20 Lines • Show All 311 Lines • ▼ Show 20 Lines (navigationState: ?PossiblyStaleNavigationState) => {
// Finally, we want to disable the swipe gesture if there is a stack with // Finally, we want to disable the swipe gesture if there is a stack with
// more than one subroute, since then the stack will have its own swipe // more than one subroute, since then the stack will have its own swipe
// gesture that will conflict with the drawer's // gesture that will conflict with the drawer's
return currentTabSubrouteState.routes.length < 2; return currentTabSubrouteState.routes.length < 2;
}, },
); );
const getChatNavState: (
navigationState: ?PossiblyStaleNavigationState,
) => ?PossiblyStaleNavigationState = (
navigationState: ?PossiblyStaleNavigationState,
) => {
tomekUnsubmitted
Done
Inline Actions
},
);
- const getChatNavState: (
- navigationState: ?PossiblyStaleNavigationState,
- ) => ?PossiblyStaleNavigationState = (
- navigationState: ?PossiblyStaleNavigationState,
- ) => {
+ function getChatNavState(navigationState: ?PossiblyStaleNavigationState): ?PossiblyStaleNavigationState {
if (!navigationState) {

It might be more readable using function syntax

tomek: It might be more readable using `function` syntax
if (!navigationState) {
return null;
}
const [firstAppSubroute] = navigationState.routes;
if (firstAppSubroute.name !== CommunityDrawerNavigatorRouteName) {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Usually the first route is the AppRouteName. Why is this different?

inka: Usually the first route is the `AppRouteName`. Why is this different?
kubaAuthorUnsubmitted
Done
Inline Actions

It is called from`OverlayRouter`, so it is nested.

kuba: It is called from`OverlayRouter`, so it is nested.
const communityDrawerState = getStateFromNavigatorRoute(firstAppSubroute);
const [firstCommunityDrawerSubroute] = communityDrawerState.routes;
if (firstCommunityDrawerSubroute.name !== TabNavigatorRouteName) {
return null;
}
const tabState = getStateFromNavigatorRoute(firstCommunityDrawerSubroute);
if (!tabState) {
return null;
}
let chatRoute;
for (const route of tabState.routes) {
if (route.name === ChatRouteName) {
chatRoute = route;
break;
}
}
if (!chatRoute || !chatRoute.state) {
return null;
}
const chatRouteState = getStateFromNavigatorRoute(chatRoute);
if (chatRouteState.type !== 'stack') {
return null;
}
inkaUnsubmitted
Not Done
Inline Actions

Why is this necessary? When can chatRouteState.type not be 'stack'? Or is this something flow requires?

inka: Why is this necessary? When can `chatRouteState.type` not be `'stack'`? Or is this something…
kubaAuthorUnsubmitted
Done
Inline Actions

This function is based on drawerSwipeEnabledSelector function. It might be unnecessary. Not sure if I should remove it.

kuba: This function is based on `drawerSwipeEnabledSelector` function. It might be unnecessary. Not…
return chatRouteState;
};
const getRemoveEditMode: ( const getRemoveEditMode: (
chatRouteState: ?PossiblyStaleNavigationState, chatRouteState: ?PossiblyStaleNavigationState,
) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => { ) => ?RemoveEditMode = (chatRouteState: ?PossiblyStaleNavigationState) => {
if (!chatRouteState) { if (!chatRouteState) {
return null; return null;
} }
const messageListRoute = const messageListRoute =
chatRouteState.routes[chatRouteState.routes.length - 1]; chatRouteState.routes[chatRouteState.routes.length - 1];
Show All 29 Linesexport {
useActiveMessageList, useActiveMessageList,
calendarActiveSelector, calendarActiveSelector,
nativeCalendarQuery, nativeCalendarQuery,
nonThreadCalendarQuery, nonThreadCalendarQuery,
useCalendarQuery, useCalendarQuery,
drawerSwipeEnabledSelector, drawerSwipeEnabledSelector,
useCurrentLeafRouteName, useCurrentLeafRouteName,
getRemoveEditMode, getRemoveEditMode,
getChatNavState,
}; };
View Options

web/chat/chat-thread-list-see-more-sidebars.react.js

Loading...