Page MenuHomePhabricator

[native] Block TimeStamp from rendering in the pinned messages screen
ClosedPublic

Authored by rohan on Jun 5 2023, 3:45 PM.
Tags
None
Referenced Files
F1568575: D8098.id27659.diff
Thu, Apr 18, 1:38 AM
F1568574: D8098.id27525.diff
Thu, Apr 18, 1:38 AM
F1568573: D8098.id27449.diff
Thu, Apr 18, 1:38 AM
F1568553: D8098.id.diff
Thu, Apr 18, 1:38 AM
F1568534: D8098.diff
Thu, Apr 18, 1:35 AM
Unknown Object (File)
Mar 5 2024, 9:23 PM
Unknown Object (File)
Feb 23 2024, 7:45 PM
Unknown Object (File)
Feb 23 2024, 7:31 PM
Subscribers

Details

Summary

As mentioned in ENG-3959, the TimeStamp renders through the MessageHeader in screens that are not MessageList, causing it to be blocked off by the header.

Based on the designs, we shouldn't render this in the first place.

Test Plan

Confirm that timestamps still show in chat, but don't display onPress in the MessageResultsScreen.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan edited the test plan for this revision. (Show Details)
native/chat/message-header.react.js
55–61 ↗(On Diff #27449)

Not sure if this is the best use case for an IIFE, but it made it simpler than writing a lot of flow-checks in line:

if (
    route.name === MessageListRouteName ||
    (typeof route.params?.presentedFrom === 'string' &&
      route.params.presentedFrom.startsWith(MessageListRouteName))
)

Separately, I check presentedFrom.startsWith instead of presentedFrom === since the presentedFrom value is the navigation key, not just the route name. So it'll be something like MessageList83918

rohan requested review of this revision.Jun 5 2023, 4:03 PM
ginsu requested changes to this revision.Jun 6 2023, 8:33 AM

questions regarding use of IIFE

native/chat/message-header.react.js
55–61 ↗(On Diff #27449)

Not sure if using an IIFE here is what we want to do. Did a quick global search through our codebase and it seems like we use IIFEs mostly to execute async functions. Here are other use cases for IIFEs but I don't think what we are doing up here falls under one of those buckets

This revision now requires changes to proceed.Jun 6 2023, 8:33 AM
native/chat/message-header.react.js
55–61 ↗(On Diff #27449)

What's the downside of using the IIFE here though?

native/chat/message-header.react.js
55–61 ↗(On Diff #27449)

The IIFE is also going to get created/called on every re-render. I feel like it would be simpler/easier to understand to instead break this down into two separate expressions, maybe something like:

const validType = typeof route.params?.presentedFrom === 'string'; // not the best variable name here
const presentedFromMessageList = validType && route.params.presentedFrom.startsWith(MessageListRouteName);
native/chat/message-header.react.js
55–61 ↗(On Diff #27449)

Ok that's fair, I only went down this route since I thought it was slightly cleaner. I'll update this

This revision is now accepted and ready to land.Jun 12 2023, 6:35 AM