[PAID] [$250] Migrate index.js to function component

[marcaaron]

Class Component Migration

Filenames

• src/components/DisplayNames/index.js
  • type: React.PureComponent
  • has state values: true
  • has refs: true
  • has context: false
  • lifecycle methods: componentDidMount

Task

• We currently have some class components in our codebase that we would like to refactor to a function component.
• Here's a link with some general advice on how to refactor a class component to a function component: https://react.dev/reference/react/Component#alternatives
• If you need additional guidance, please ask in #expensify-open-source
• Test for any regressions and verify that there are no breaking changes

[MelvinBot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[muxriddinmuqimov77]

I'd like to work on this issue

[s-alves10]

I'd love to work on this.

[marcaaron]

Taking this one off HOLD.

[gedu]

Hey, I am Edu from Callstack - an expert contributor group.

I can work on this one

[gedu]

@marcaaron I found a bug, tried to look at it on Slack but couldn't find it.

Will be an easy fix, what should I do? hold on to the migration and fix this into another PR or should I fix it here? The fix will be in another file not related to the migration file.

[marcaaron]

I see @srikarparsi is assigned to this issue. Have they handed it off to you @gedu and should we update the assignment to reflect?

I think it's really up to you and maybe relative to the complexity of this refactor? If it is a small change overall then fixing should be ok. Might want to check to ensure there are no open issues or people working on the bug in parallel. Usually with the obvious bugs people are quite fast to report them.

[gedu]

@marcaaron I just saw that was assigned a month ago, didn't notice that he was working on it already, my bad.

I wouldn't recommend refactoring this one-to-one from a class component to a functional component. There are some areas that can be improved, and we need to ensure that it doesn't have any negative impact in the future.

On the other hand, the bug fixing this issue is really easy.

[marcaaron]

There are some areas that can be improved, and we need to ensure that it doesn't have any negative impact in the future.

Thanks very much, can you elaborate on the specific concern and which areas could be improved?

On the other hand, the bug fixing this issue is really easy.

Got it. I think if you have found a bug then let's separate it from the refactor for now and do a small quick PR?

@srikarparsi can you sync up with @gedu and make sure we aren't doubling efforts on this one?

[gedu]

@marcaaron Yes.

Firstly, there is an if to handle a DisplayNames with and without Tooltip, I would split it into 2 components because the one without tooltip doesn't need any function, refs, or states initialization.
Secondly, the function getTooltipShiftX must be in a useCallback because deeper in the component UserDetailsTooltip it uses an useMemo so we need that function to be stable. In addition, that function is being called using a lambda function shiftHorizontal={() => getTooltipShiftX(index)}, so we must not use the lambda function and make it stable too.
Lastly, but optional, I would separate into files the different DisplayNames with/without tooltip, just to make the code more readable and avoid having all the logic in the index.js file.

Cool, I could post this bug into the expensify-bugs channel

[srikarparsi]

Hi @gedu! Sorry for the late response, I was out of office yesterday and most of the day before for July 4th. I worked on the issue a while back and put up a draft PR but didn't get around to finishing it. I can make changes to my draft PR based on your comment. Would you be willing to review my changes?

[gedu]

Hey, @srikarparsi, no problem, I can review the PR

[melvin-bot]

This issue has not been updated in over 15 days. @srikarparsi eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[melvin-bot]

This issue has not been updated in over 15 days. @srikarparsi eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[aimane-chnaif]

@srikarparsi can you please add Bug label for payment?

[aimane-chnaif]

@srikarparsi can you please add Bug label for payment?

bump ^

[melvin-bot]

@srikarparsi, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

[aimane-chnaif]

@marcaaron can you please add Bug label for payment? Thanks

[melvin-bot]

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[strepanier03]

Looking at this for payment.

Am I correct in assuming this is $250 payment to both @srikarparsi and @aimane-chnaif?

@marcaaron? Do you have any context into the payment here? Once I know final amounts and who to pay I can make an Upwork job and hire/pay out.

[aimane-chnaif]

@srikarparsi is engineer 😄
I am only the one for upwork payment

[strepanier03]

Hahaha you're right @aimane-chnaif, what a Monday yesterday was 😂.

I'll make the job and hire you to it shortly.

[strepanier03]

All right, well I'll try again in a bit to make a job. Upwork is giving me errors no matter how I try to create a job right now. :(

[strepanier03]

@aimane-chnaif - I finally got that job created and hired you to it, sorry for the delay on it, sometimes Upwork is frustrating to use.

I'll check it later today to pay.

[strepanier03]

I've paid this, closed the contract, and submitted the form for the contact list. This is all set now and I'll close it out.