Details
flow and this will be further tested in a subsequent diff
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
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 | 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 | 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 | Unfortunately, flow can't pick up default values, so went with the first suggestion. |