Page MenuHomePhabricator

[web] introduce thread settings utils
ClosedPublic

Authored by ginsu on Feb 12 2024, 1:43 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 31, 6:25 AM
Unknown Object (File)
Oct 3 2024, 3:03 PM
Unknown Object (File)
Oct 3 2024, 3:03 PM
Unknown Object (File)
Oct 3 2024, 3:03 PM
Unknown Object (File)
Oct 3 2024, 3:03 PM
Unknown Object (File)
Oct 3 2024, 3:02 PM
Unknown Object (File)
Sep 14 2024, 9:55 PM
Unknown Object (File)
Aug 25 2024, 2:00 PM
Subscribers

Details

Summary

Move diff, I created this file to house all the business logic for the thread settings modal into separate hooks. We need to do this because we need to use this logic outside of this component and use it in a "primaryButton" prop that we will pass to the modal, and I thought having this logic preemptively in hooks would make the code more readable when we introduce the necessary primaryButton

Linear task: https://linear.app/comm/issue/ENG-5943/extendmodify-the-modal-props-api-to-follow-new-modal-designs

Depends on D11030

Test Plan

flow + confirmed that the thread settings modal still works as expected

general:

privacy:

Delete:

error:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu added reviewers: atul, inka.
ginsu edited the summary of this revision. (Show Details)
ginsu requested review of this revision.Feb 12 2024, 1:57 AM
atul requested changes to this revision.Feb 12 2024, 12:47 PM

Seems reasonable, but going to have to be annoying here and request a more fleshed out Test Plan for this.

No need for screenshots/screen recordings, but would be good to list out flows you tested and try to reproduce some error states (maybe by killing keyserver)?

Especially important since this will be landed all at once with a bunch of other changes, so should try to make sure each of these are tested thoroughly

This revision now requires changes to proceed.Feb 12 2024, 12:47 PM
ginsu edited the test plan for this revision. (Show Details)

updated test plan

Appreciate that you took the time to update Test Plan, it looks great.

This revision is now accepted and ready to land.Feb 14 2024, 2:13 PM
This revision was landed with ongoing or failed builds.Feb 15 2024, 2:26 AM
This revision was automatically updated to reflect the committed changes.