Details
flow and this will be further tested in a subsequent diff
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
We're copying a lot of complicated logic. Can we reuse it in a different way?
| native/navigation/tooltip.react.js | ||
|---|---|---|
| 444–445 ↗ | (On Diff #21547) | Why we're using this strange syntax? |
put reactionSelectionPopoverContainerStyle logic into a hook called useReactionSelectionPopoverPosition
An approach looks better, but we're duplicating a lot of logic. We should reuse it by e.g. introducing functions with common parts. Accepting, to not block the goal, but please improve this later.
| native/chat/reaction-message-utils.js | ||
|---|---|---|
| 132–146 ↗ | (On Diff #21709) | Why do we use such complicated syntax? Was it copied from somewhere? The margin calculation can be replaced by just const calculatedMargin = margin ?? 16; But we can simplify it one step further by using a default value. |
| 155 ↗ | (On Diff #21709) | Is it really beneficial to introduce this variable? |
An approach looks better, but we're duplicating a lot of logic. We should reuse it by e.g. introducing functions with common parts. Accepting, to not block the goal, but please improve this later.
| native/chat/reaction-message-utils.js | ||
|---|---|---|
| 132–146 ↗ | (On Diff #21709) | Unfortunately, flow can't pick up default values, so went with the first suggestion. |