Details

Reviewers

atul
ashoat

Commits

Summary

make lastActivity change when thread is updated

sup nerds thread changes the top time from 11:36pm to 11:37pm

Test Plan

update a thread in a chat, time should change.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

web/chat/chat-thread-list-item.react.js
28 ↗	(On Diff #9537)	Not sure why, when I try adding this hook to: `lib/shared/thread-utils` `timeZone` isn't in `state` I would have put this hook in there, if I wasn't getting a flow error. @atul

• benschac requested review of this revision.Feb 10 2022, 8:36 AM

web/chat/chat-thread-list-item.react.js
28 ↗	(On Diff #9537)	A different `useSelector` is used. Here it is `import { useSelector } from '../redux/redux-utils';`, but in `lib/shared/thread-utils` it is `import { useSelector } from '../utils/redux-utils';`

move hook to lib

lib/shared/thread-utils.js
1181 ↗	(On Diff #9543)	I tried fiiltering + map'ing without any luck. Assuming there's a way to narrow the type.

ashoat requested changes to this revision.Feb 10 2022, 8:50 PM

ashoat added inline comments.

lib/shared/thread-utils.js
1180 ↗	(On Diff #9543)	This isn't a hook, so it shouldn't start with `use` I don't think we need a new function for this. We can conclude the code already exists because we need to calculate this number in order to sort the list of top-level threads. Given that, the first thing you should've done here was to find where the existing code is. And if you found some reason we can't reuse it, you should anticipate that your reviewer is going to ask this question, and preemptively discuss it to save everybody some time
1181 ↗	(On Diff #9543)	Not sure what you're talking about here. Anyways, I'm guessing it's not relevant if we can find some code to reuse. If I'm wrong about the code reuse, please try to explain your question in more detail. @benschac, at a high level I'd suggest spending about 3-5x as much time as you currently spend on communication. Every time you need to communicate something to somebody, imagine you are the person on the receiving end and consider whether you are communicating all the information for the person on the receiving end to be able to understand what you're talking about.

This revision now requires changes to proceed.Feb 10 2022, 8:50 PM

remove unneeded function. Use functionality that exists

remove line

lib/shared/thread-utils.js
1180 ↗	(On Diff #9543)	Sorry, I should have caught that. It was at one point. That also makes sense. I didn't think of it that way but it clicked after reading this comment.
1181 ↗	(On Diff #9543)	@benschac, at a high level I'd suggest spending about 3-5x as much time as you currently spend on communication. Every time you need to communicate something to somebody, imagine you are the person on the receiving end and consider whether you are communicating all the information for the person on the receiving end to be able to understand what you're talking about. That's fair.