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
F3488477: D8098.diff
Wed, Dec 18, 10:05 AM
Unknown Object (File)
Sat, Dec 7, 7:32 AM
Unknown Object (File)
Nov 6 2024, 3:46 PM
Unknown Object (File)
Oct 31 2024, 1:05 PM
Unknown Object (File)
Oct 28 2024, 12:37 PM
Unknown Object (File)
Oct 23 2024, 4:15 PM
Unknown Object (File)
Sep 29 2024, 8:41 AM
Unknown Object (File)
Sep 29 2024, 8:41 AM
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
No Lint Coverage
Unit
No Test Coverage

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