Page MenuHomePhabricator

[web] Push "Delete thread" functionality down to `ThreadSettingsDeleteTab`
ClosedPublic

Authored by atul on Apr 25 2022, 10:34 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:22 PM
Unknown Object (File)
Thu, Dec 19, 1:20 PM
Unknown Object (File)
Wed, Nov 27, 3:14 AM

Details

Summary

Push "Delete thread" functionality (onDelete & deleteThreadAction) down to ThreadSettingsDeleteTab component.

Also made necessary "supporting changes":

  • Move "Delete" button to ThreadSettingsDeleteTab
  • Pass additional props to ThreadSettingsDeleteTab so deleteThreadAction and onDelete could be "recreated" from within the child component

(This component is definitely in need of restyling, but will sequence that a bit later in ThreadSettingsModal work)


Depends on D3832

Test Plan
  1. Open ThreadSettingsModal
  2. Navigate to the "Delete" tab
  3. Make sure that everything continues to look as expected

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Apr 25 2022, 10:53 AM
web/modals/threads/thread-settings-modal.react.js
226 ↗(On Diff #11883)

Going to move the "button" for General and Privacy tab into their respective Tab components in subsequent diff

This does mean we may have to "repeat ourselves" with changeThreadSettingsAction in two components, but I think that's a okay tradeoff and leads to overall cleaner abstraction

tomek added inline comments.
web/modals/threads/thread-settings-modal.react.js
226 ↗(On Diff #11883)

To avoid the repetition, we probably can create a new component that contains the logic and is used on these both cards

This revision is now accepted and ready to land.Apr 26 2022, 8:47 AM
benschac added inline comments.
web/modals/threads/thread-settings-delete-tab.react.js
52 ↗(On Diff #11952)

@atul I think this should be unknown?

web/modals/threads/thread-settings-delete-tab.react.js
52 ↗(On Diff #11952)

Yes! Thanks for catching that. Going to do a quick sweep through the other strings in ThreadSettingsModal and the children Tab components to make sure there aren't any other typos.