Context: here
Fix transition bug that occurred during uploading image on chats.
Details
Details
Tested the fix on Android and iOS, both virtual and physical device and it works correctly.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- jakub/temporary
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
The ShellCheck CI build is failing because you did not rebase this diff on top of master when you submitted it for review. If you rebase this diff on top of the latest changes from the repository, that build should pass.
I think the iOS build will also fix if you rebase on top of master, but not sure.
Also just for good measure, when you do pull from master and rebase this diff, make sure you run the Test Plan again so you know everything still works, since technically you were running your changes in an old version of the repository.
And great job on figuring out this bug!!
Comment Actions
I'd love to understand why this component initially renders with !triggerReply && !triggerSidebar in the case of non-sidebars. (This is the question I was trying to ask on Linear.)
@jakub, before we accept this, can you "get to the bottom of this" and figure out what's going on there? Either way, the change in this diff probably makes sense... but it seems like there might be a "deeper issue" we need to resolve.
| native/chat/swipeable-message.react.js | ||
|---|---|---|
| 251 ↗ | (On Diff #15510) | Is there any way we can "disable" this component to reduce the performance impact of using it when we don't need it? Perhaps there is a property like enabled={false} or something. Can you check the docs? |
| 252 ↗ | (On Diff #15510) | Why did we add transformMessageBoxStyle here? |
| native/chat/swipeable-message.react.js | ||
|---|---|---|
| 251 ↗ | (On Diff #15510) | It would've been helpful to link the docs after updating the diff. I initially wasn't sure if you just added the enabled={false} because I suggested it, versus actually researching the docs and figuring out what it should be. As a result, I ended up having to do the research myself, which wasn't a great use of my time. It helps to respond explicitly to review comments that ask you to do X (eg. read the docs) with the steps you followed and any relevant links :) |
| native/chat/swipeable-message.react.js | ||
|---|---|---|
| 251 ↗ | (On Diff #15510) | We can disable handling gestures using enabled property as described here, but it doesn't follow clearly from docs that it could reduce performance impact. |
| 252 ↗ | (On Diff #15510) | I think it's unnecessary and we could remove it |