Page MenuHomePhabricator

[native] update ThreadSettingsPushNotifs to use new thread notifs settings experience
ClosedPublic

Authored by ginsu on Jul 4 2024, 12:12 AM.
Tags
None
Referenced Files
F3568471: D12665.id.diff
Fri, Dec 27, 10:50 PM
Unknown Object (File)
Sun, Dec 22, 11:08 PM
Unknown Object (File)
Nov 27 2024, 9:19 PM
Unknown Object (File)
Nov 27 2024, 8:29 PM
Unknown Object (File)
Nov 24 2024, 3:54 AM
Unknown Object (File)
Nov 24 2024, 3:26 AM
Unknown Object (File)
Nov 24 2024, 1:33 AM
Unknown Object (File)
Nov 10 2024, 4:43 PM
Subscribers

Details

Summary

Currently the ThreadSettingsPushNotifs component renders a switch that the user can use to update their notif settings. We should update ThreadSettingsPushNotifs to instead navigate to our new thread settings notifications screen

Depends on D12664

Test Plan

Please see the demo video below

When hasPushPermissions is true:

When hasPushPermissions is false:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

native/chat/settings/thread-settings.react.js
462 ↗(On Diff #42001)

Updated the copy to match what we have on web

Screenshot 2024-07-04 at 3.18.54 AM.png (146×280 px, 10 KB)

native/components/edit-setting-button.react.js
12 ↗(On Diff #42001)

I made this optional to prevent some redundancy in ThreadSettingsPushNotifs

ginsu requested review of this revision.Jul 4 2024, 12:30 AM
ashoat added inline comments.
native/chat/settings/thread-settings-push-notifs.react.js
48–50 ↗(On Diff #42001)

This constructor can be removed now. It's not doing anything except calling its parent. If it's not defined, the parent will be called anyways

60–66 ↗(On Diff #42001)

Let's avoid ternary like this. See point 3 here

Instead I think it would be cleaner to assign two variables and render both below:

let editSettingsButton, notifSettingsLinkingButton;
if (this.props.hasPushPermissions) {
  editSettingsButton = (
    <EditSettingButton onPress={this.onPressEditThreadNotificationSettings} />
  );
} else {
  notifSettingsLinkingButton = (
    <TouchableOpacity onPress={this.onNotificationsSettingsLinkingIconPress}>
      <SWMansionIcon name="info-circle" size={20} color="gray" />
    </TouchableOpacity>
  );
}
This revision is now accepted and ready to land.Jul 4 2024, 6:40 AM

address comments + rebase before landing

This revision was landed with ongoing or failed builds.Jul 4 2024, 11:21 AM
This revision was automatically updated to reflect the committed changes.