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
F3529943: D7446.id25670.diff
Tue, Dec 24, 10:23 PM
F3529939: D7446.id25253.diff
Tue, Dec 24, 10:21 PM
F3528739: D7446.diff
Tue, Dec 24, 10:02 AM
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)
Nov 24 2024, 2:27 PM
Unknown Object (File)
Nov 8 2024, 5:05 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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

native/chat/chat-input-bar.react.js
892–901

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

Can we use Unicode apostrophe here?

915

Can't you do onPress: saveExit?

native/chat/chat-input-bar.react.js
190

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