Page MenuHomePhabricator

[web] Introduce minimally viable `ThreadSettingsPrivacyTab` component
ClosedPublic

Authored by atul on Apr 12 2022, 8:19 PM.
Tags
None
Referenced Files
F2210884: D3717.id.diff
Mon, Jul 8, 12:29 AM
F2210449: D3717.id11627.diff
Sun, Jul 7, 10:35 PM
Unknown Object (File)
Tue, Jul 2, 8:39 PM
Unknown Object (File)
Sun, Jun 30, 12:35 AM
Unknown Object (File)
Sun, Jun 23, 11:10 AM
Unknown Object (File)
Sat, Jun 22, 12:48 PM
Unknown Object (File)
Thu, Jun 20, 11:25 PM
Unknown Object (File)
Wed, Jun 19, 1:49 PM

Details

Summary

In an effort to make ThreadSettingsModal easier to work with, we're pulling general, privacy, and delete tabs out into their own separate components.

This diff does that for ThreadSettingsPrivacyTab, but in a limited way. We're basically moving over the mainContent JSX markup and prop drilling that state that's still held by ThreadSettingsModal.

Once we've extracted the JSX markup for general, privacy, and delete.. and prop drilled as necessary to keep things working as they did before: we're going to push down the state and functionality into the smaller "tab" components so things are properly encapsulated.

And once that's done, we should easily be able to turn ThreadSettingsModal into a functional component.. and refactor ThreadSettingsModal using @def-au1t's TabsContainer and TabsItem components.


Depends on D3716

Test Plan

This diff should be a noop. I spend some time changing the thread privacy settings--and things continue to look and work as expected (we'll likely need a restyle of this tab, but that's for another diff):

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul requested review of this revision.Apr 12 2022, 8:25 PM
atul retitled this revision from [web] Introduce minimally viable `ThreadSettingsPrivacyTab` to [web] Introduce minimally viable `ThreadSettingsPrivacyTab` component.Apr 12 2022, 8:38 PM
tomek requested changes to this revision.Apr 14 2022, 2:10 AM
tomek added inline comments.
web/modals/threads/thread-settings-privacy-tab.css
1–37 ↗(On Diff #11377)

This style has a couple of issues, e.g. we're not using variables. Was it copied from somewhere?

we'll likely need a restyle of this tab, but that's for another diff

So I guess, we're going to replace most of this code.Wondering, if it would be better if we just used the existing styles without copying it. What do you think?

web/modals/threads/thread-settings-privacy-tab.react.js
26 ↗(On Diff #11377)

thread-settings-privacy-tab.css doesn't have this class and uses underscores instead of dashes

This revision now requires changes to proceed.Apr 14 2022, 2:10 AM
atul requested review of this revision.Apr 15 2022, 12:57 PM
atul added inline comments.
web/modals/threads/thread-settings-privacy-tab.css
1–37 ↗(On Diff #11377)

Yeah this was just straight copied from thread-settings-modal.css for now.

The plan is

  1. Move the "tab-specific" styles to component-specific stylesheets.
  2. Remove the "tab-specific" styles from thread-settings-modal.css
  3. Turn ThreadSettingsModal into a functional component (if that doesn't turn out to be too crazy)
  4. Clean up the styles in the ThreadSettingsBlahTab components (use variables, fill in missing selectors eg modal-radio-selector, etc)

I'll put up a diff right now that removes these styles from thread-settings-modal.css so we don't have redundancy.

web/modals/threads/thread-settings-privacy-tab.react.js
26 ↗(On Diff #11377)

Yeah, this modal-radio-selector has been missing for a while.

Intentionally left it using dashes to make it obvious when it comes time to fix the styling for this component

web/modals/threads/thread-settings-privacy-tab.css
1–37 ↗(On Diff #11377)

I'll put up a diff right now that removes these styles from thread-settings-modal.css so we don't have redundancy.

D3749

This revision is now accepted and ready to land.Apr 19 2022, 2:40 AM
This revision was landed with ongoing or failed builds.Apr 19 2022, 10:51 AM
This revision was automatically updated to reflect the committed changes.