Page MenuHomePhabricator
Differential D3837

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

Authored by atul on Apr 25 2022, 10:34 AM.
Tags
None
Referenced Files
F3175239: D3837.diff
Thu, Nov 7, 7:39 PM
Unknown Object (File)
Tue, Nov 5, 4:56 AM
Unknown Object (File)
Fri, Oct 11, 7:02 PM
Unknown Object (File)
Fri, Oct 11, 7:01 PM
Unknown Object (File)
Fri, Oct 11, 7:00 PM
Unknown Object (File)
Fri, Oct 11, 6:59 PM
Unknown Object (File)
Thu, Oct 10, 2:32 AM
Unknown Object (File)
Thu, Oct 10, 2:32 AM
View All Files
Subscribers
• abosh
• adrian
ashoat
• benschac
• karol

Details

Reviewers
• jacek
tomek
Commits
rCOMMf6a855db5172: [web] Push "Delete thread" functionality down to `ThreadSettingsDeleteTab`
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
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.Apr 25 2022, 10:34 AM
Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 25 2022, 10:34 AM
atul mentioned this in D3783: [web] Move `deleteThreadAction` up from `ThreadSettingsModal` to `ConnectedThreadSettingsModal`.Apr 25 2022, 10:39 AM
Harbormaster completed remote builds in B8524: Diff 11883.Apr 25 2022, 10:53 AM
atul requested review of this revision.Apr 25 2022, 10:53 AM
atul added inline comments.Apr 25 2022, 11:02 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

atul added a child revision: D3838: [web] Add `accountPasswordInputRef` back to `ThreadSettingsDeleteTab`.Apr 25 2022, 11:18 AM
tomek accepted this revision.Apr 26 2022, 8:47 AM
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
atul updated this revision to Diff 11952.Apr 26 2022, 10:41 AM

rebase

• benschac added a subscriber: • benschac.Apr 26 2022, 10:43 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?

Harbormaster completed remote builds in B8573: Diff 11952.Apr 26 2022, 10:48 AM
atul added inline comments.Apr 26 2022, 11:11 AM
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.

atul updated this revision to Diff 11958.Apr 26 2022, 11:13 AM

fix typo

Harbormaster failed remote builds in B8579: Diff 11958!Apr 26 2022, 11:18 AM
Harbormaster completed remote builds in B8579: Diff 11958.Apr 26 2022, 11:26 AM
atul updated this revision to Diff 12081.Apr 28 2022, 2:17 PM

rebase before landing`

Harbormaster completed remote builds in B8676: Diff 12081.Apr 28 2022, 2:21 PM
Closed by commit rCOMMf6a855db5172: [web] Push "Delete thread" functionality down to `ThreadSettingsDeleteTab` (authored by atul). · Explain WhyApr 28 2022, 2:22 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMf6a855db5172: [web] Push "Delete thread" functionality down to `ThreadSettingsDeleteTab`.

Revision Contents
Changeset List

Diff 11958

View Options

web/modals/threads/thread-settings-delete-tab.react.js

Loading...
View Options

web/modals/threads/thread-settings-modal.react.js

Loading...