Page MenuHomePhabricator
Differential D8596

[web] Handle unsaved changes with the UnsavedChangesModal
ClosedPublic
Actions

Authored by rohan on Jul 21 2023, 9:23 AM.
Tags
None
Referenced Files
F5056081: D8596.id28946.diff
Tue, Mar 25, 8:08 PM
Unknown Object (File)
Mon, Mar 24, 7:20 AM
Unknown Object (File)
Sun, Mar 23, 4:40 AM
Unknown Object (File)
Tue, Mar 11, 12:21 AM
Unknown Object (File)
Feb 14 2025, 5:51 PM
Unknown Object (File)
Feb 14 2025, 5:51 PM
Unknown Object (File)
Feb 14 2025, 5:51 PM
Unknown Object (File)
Feb 14 2025, 5:51 PM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
atul
ginsu
Commits
rCOMM45ceb67981c8: [web] Handle unsaved changes with the UnsavedChangesModal
Summary

If there are any changes in the create role screen, we want to push the UnsavedChangesModal if a user tries to click "Back" or the X button on the Modal. I had to check elements individually for the rolePermissions / pendingRolePermissions array since after selecting a permission and then deselecting it, simply using a === would no longer work on the two arrays since they would not be of the same reference.

ENG-4428

Depends on D8595

Test Plan

Tested the modal to confirm that the UnsavedChangesModal appears when it should

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

rohan created this revision.Jul 21 2023, 9:23 AM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 21 2023, 9:23 AM
rohan edited the summary of this revision. (Show Details)Jul 21 2023, 9:25 AM
rohan edited the test plan for this revision. (Show Details)
rohan added a child revision: D8597: [web] Introduce client-side error handling to prevent duplicate role names.Jul 21 2023, 9:27 AM
Harbormaster completed remote builds in B21124: Diff 28946.Jul 21 2023, 9:47 AM
rohan requested review of this revision.Jul 21 2023, 9:47 AM
rohan updated this revision to Diff 28975.EditedJul 24 2023, 9:49 AM

Update check to short-circuit if array lengths are different (this is to prevent a case in the edit-role flow where a role has 3 permissions and a user deselects one, every element in the the new permissions (array size 2) is still contained in the original permissions (array size 3)

Harbormaster completed remote builds in B21153: Diff 28975.Jul 24 2023, 10:51 AM
atul requested changes to this revision.Jul 26 2023, 10:49 AM

Can we go with O(n) approach instead of O(n^2)?

web/roles/create-roles-modal.react.js
60–62 ↗(On Diff #28975)

The algorithm being used here is O(n^2). Could we go with something like:

const pendingSet = new Set(pendingRolePermissions);
const roleSet = new Set(rolePermissions);
if (pendingSet.size !== roleSet.size) {
    arePermissionsEqual= false;
}
for (let permission of pendingSet) {
    if (!roleSet.has(permission)) return false;
}
arePermissionsEqual = true;

which is O(n) without much more effort? Given the number of permissions at the moment it's probably fine as is, but might as well reduce the complexity right off the bat

This revision now requires changes to proceed.Jul 26 2023, 10:49 AM
rohan updated this revision to Diff 29068.Jul 26 2023, 11:12 AM

Implement O(n) feedback

web/roles/create-roles-modal.react.js
60–62 ↗(On Diff #28975)

Ah yeah that's probably a better idea anyways

atul accepted this revision.Jul 26 2023, 11:13 AM

Sweet, thanks for updating so quickly

This revision is now accepted and ready to land.Jul 26 2023, 11:13 AM
Harbormaster completed remote builds in B21224: Diff 29068.Jul 26 2023, 11:58 AM
rohan updated this revision to Diff 29340.Jul 31 2023, 12:06 PM

Rebase

This revision was landed with ongoing or failed builds.Jul 31 2023, 6:15 PM
Closed by commit rCOMM45ceb67981c8: [web] Handle unsaved changes with the UnsavedChangesModal (authored by rohan). · Explain Why
This revision was automatically updated to reflect the committed changes.
rohan added a commit: rCOMM45ceb67981c8: [web] Handle unsaved changes with the UnsavedChangesModal.
Harbormaster completed remote builds in B21428: Diff 29340.Jul 31 2023, 9:17 PM

Revision Contents
Changeset List

PathSize
web/
roles/
26 lines

Diff 28946

View Options

web/roles/create-roles-modal.react.js

Show All 40 Linesfunction ManagePublicLinkScreen(props: Props): React.Node {
let errorComponent = null; let errorComponent = null;
if (error) { if (error) {
errorComponent = <Text style={styles.error}>{error}</Text>; errorComponent = <Text style={styles.error}>{error}</Text>;
} }
const onDisableButtonClick = React.useCallback(() => { const onDisableButtonClick = React.useCallback(() => {
Alert.alert( Alert.alert(
'Disable public link', 'Disable public link',
'Are you sure you want to disable your public link? Members who have your community’s public link but have not joined will not able to with the disabled link. \n' + 'Are you sure you want to disable your public link?\n' +
'\n' + '\n' +
'Other communities may also claim your previous public link url.', 'Other communities will be able to claim the same URL.',
[ [
{ {
text: 'Confirm disable', text: 'Confirm disable',
style: 'destructive', style: 'destructive',
onPress: disableInviteLink, onPress: disableInviteLink,
}, },
{ {
text: 'Cancel', text: 'Cancel',
Show All 18 Lines disablePublicLinkButton = (
</View> </View>
); );
} }
return ( return (
<View> <View>
<View style={styles.section}> <View style={styles.section}>
<Text style={styles.sectionText}> <Text style={styles.sectionText}>
Let your community be more accessible with your own unique public Invite links make it easy for your friends to join your community.
link. By enabling a public link, you are allowing anyone who has your Anybody who knows your community’s invite link will be able to join
link to join your community.{'\n\n'} it. Note that if you change your public link’s URL, other communities
Editing your community’s public link allows other communities to claim will be able to claim the old URL.
ashoatUnsubmitted
Not Done
Inline Actions

From the screenshot, this looks like too much text in a row. We could try separating out the third sentence into its own paragraph. We could also try showing the three sentences as three bullet points. @ted, any thoughts?

ashoat: From the screenshot, this looks like too much text in a row. We could try separating out the…
tomekAuthorUnsubmitted
Done
Inline Actions

In the original design, the last sentence was separated by an empty line. It probably makes sense to do the same here.

native-manage-newline.png (497×407 px, 38 KB)

tomek: In the original design, the last sentence was separated by an empty line. It probably makes…
your previous URL.
</Text> </Text>
</View> </View>
<Text style={styles.sectionTitle}>INVITE URL</Text> <Text style={styles.sectionTitle}>INVITE URL</Text>
<View style={styles.section}> <View style={styles.section}>
<View style={styles.inviteLink}> <View style={styles.inviteLink}>
<Text style={styles.inviteLinkPrefix}>{inviteLinkUrl('')}</Text> <Text style={styles.inviteLinkPrefix}>{inviteLinkUrl('')}</Text>
<TextInput <TextInput
style={styles.input} style={styles.input}
▲ Show 20 Lines • Show All 108 Lines • Show Last 20 Lines