search

MLH-Fellowship / scheduling-profiler-prototype

Custom profiler prototype for React's concurrent mode
https://react-scheduling-profiler.vercel.app/
6 stars 0 forks source link

Refactor tooltip label switch into a util #103

Closed kartikcho closed 4 years ago

kartikcho commented 4 years ago

This PR fixes #37 by moving tooltip label switch into a util.

vercel[bot] commented 4 years ago

This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/j2cvr3wp8 ✅ Preview: https://scheduling-profiler-prototype-git-tooltiplabel.mlh-fellowship.vercel.app

kartikcho commented 4 years ago

I saw some potential areas to reduce other switch cases but not sure it would help that much and is better as is.

One such example,

https://github.com/MLH-Fellowship/scheduling-profiler-prototype/blob/5fe70a3c2f85f83da80bc96dbda621364c07adc9/src/canvas/views/ReactEventsView.js#L86-L110

taneliang commented 4 years ago

@kartikcho I think that switch in ReactEventsView that you highlighted is fine as is too, but I won't be opposed if you wanted to extract that out into a function. I don't have an opinion on that :)

kartikcho commented 4 years ago

Is there any reason why the code should be extracted into its own file? Since these functions are only used in one place they should remain in the same file in my opinion.

That's a valid point, my initial assumption was going to make one file with functions for all the switch case scenarios but they're better as is. I'll revert