Page MenuHomePhabricator

[native] Discard alert when navigating in edit mode
ClosedPublic

Authored by kuba on Apr 14 2023, 1:24 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 9, 4:14 PM
Unknown Object (File)
Thu, Dec 5, 2:27 AM
Unknown Object (File)
Sun, Dec 1, 1:21 AM
Unknown Object (File)
Sun, Nov 24, 2:27 PM
Unknown Object (File)
Nov 8 2024, 5:05 PM
Unknown Object (File)
Nov 8 2024, 5:05 PM
Unknown Object (File)
Nov 8 2024, 5:05 PM
Unknown Object (File)
Nov 8 2024, 5:04 PM
Subscribers

Details

Summary

When we are in edit mode we don't want to lose changes by navigating. I display an alert when we are in edit mode and we have some changes in the edited message.

Test Plan

Run the app on iOS, and check if in edit mode with changes when going back to the thread list, the alert is displayed, and if works as expected.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/chat/chat-input-bar.react.js
892–901 ↗(On Diff #25157)

I had to add the saveExit method to preserve the draft correctly. Without this, it would be overwritten with the edited message.

native/chat/chat-input-bar.react.js
911 ↗(On Diff #25157)

Can we use Unicode apostrophe here?

915 ↗(On Diff #25157)

Can't you do onPress: saveExit?

native/chat/chat-input-bar.react.js
190 ↗(On Diff #25157)

Some other name like clearBeforeRemoveListener would be clearer

kuba marked 3 inline comments as done.

Addressed review

native/chat/chat-input-bar.react.js
1102 ↗(On Diff #25253)

Instead of passing it in, would it be possible to useNavigation to get the navigation prop?

Not sure it makes sense, just a thought

native/chat/chat-input-bar.react.js
1102 ↗(On Diff #25253)

Yes, it is possible. I can change it if you want. But I think it isn't necessary, because it is only one "passing". Should I change it?

native/chat/chat-input-bar.react.js
1102 ↗(On Diff #25253)

Up to you, I don't feel strongly if there's only one instance of "prop drilling"

native/chat/chat-input-bar.react.js
1102 ↗(On Diff #25253)

In that case, I would leave it like this then.

This revision is now accepted and ready to land.Apr 20 2023, 5:19 AM