Page MenuHomePhabricator
Differential D4051

[web] Use `EnumSettingOption` components in `ThreadSettingsPrivacyTab`
ClosedPublic
Actions

Authored by atul on May 15 2022, 1:38 PM.
Tags
None
Referenced Files
F3119591: D4051.id12697.diff
Fri, Nov 1, 2:27 AM
F3119580: D4051.id12963.diff
Fri, Nov 1, 2:24 AM
Unknown Object (File)
Mon, Oct 28, 2:32 AM
Unknown Object (File)
Fri, Oct 25, 2:59 AM
Unknown Object (File)
Wed, Oct 23, 4:16 PM
Unknown Object (File)
Wed, Oct 23, 4:15 PM
Unknown Object (File)
Wed, Oct 23, 4:15 PM
Unknown Object (File)
Tue, Oct 22, 5:44 AM
View All Files
Subscribers
• abosh
• adrian
ashoat

Details

Reviewers
tomek
• jacek
• benschac
Commits
rCOMM231bfee83568: [web] Use `EnumSettingOption` components in `ThreadSettingsPrivacyTab`
Summary

This is a minimally-viable first pass.

Replace the inputs in ThreadSettingsPrivacyTab with EnumSettingOptions. There is still a good bit to go with stying, but should be functionally complete.

Here's how it looks:

Depends on D4050

Test Plan

Click around, change visibility, observe that things look/behave as expected

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul created this revision.May 15 2022, 1:38 PM
Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 15 2022, 1:38 PM
atul published this revision for review.May 15 2022, 1:40 PM
atul edited the summary of this revision. (Show Details)
atul added a parent revision: D4050: [web] Pass `optionSelected` from `EnumSettingsOption` to `EnumSettingsOptionInfo`.
atul added inline comments.
web/modals/threads/settings/thread-settings-privacy-tab.react.js
33

Will probably type these as maybe instead of optional in subsequent diff to avoid having to explicitly write undefined here

114

Lack of disabled prop is a "known issue," will be addressed later in this stack.

Harbormaster completed remote builds in B9131: Diff 12696.May 15 2022, 1:43 PM
atul updated this revision to Diff 12697.May 15 2022, 2:46 PM

max-width

atul edited the summary of this revision. (Show Details)May 15 2022, 2:47 PM
atul added inline comments.
web/modals/threads/settings/thread-settings-privacy-tab.react.js
104–122 ↗(On Diff #12697)

could probably introduce a third "general" callback to cut repetition here

Harbormaster completed remote builds in B9132: Diff 12697.May 15 2022, 2:53 PM
tomek accepted this revision.May 17 2022, 7:10 AM
tomek added inline comments.
web/components/enum-settings-option-info.css
5 ↗(On Diff #12697)

Shouldn't it be limited by its parent? It would become more reusable.

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

Why do we need to assert a constant?

This revision is now accepted and ready to land.May 17 2022, 7:10 AM
atul updated this revision to Diff 12961.May 19 2022, 9:00 AM

rebase after resolving merge conflicts and before landing

Harbormaster completed remote builds in B9355: Diff 12961.May 19 2022, 9:05 AM
atul added inline comments.May 19 2022, 9:07 AM
web/components/enum-settings-option-info.css
5 ↗(On Diff #12697)

Yeah, you're right that this is hacky and specifically to get ThreadSettingsPrivacyTab to look right.

Will find a better approach here that's more general

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

Good point, there's no longer a reason. We were previously asserting that event.currentTarget.value was a threadType, but that's not longer relevant. Will update.

atul updated this revision to Diff 12962.May 19 2022, 10:34 AM

address feedback

Harbormaster completed remote builds in B9356: Diff 12962.May 19 2022, 10:39 AM
Closed by commit rCOMM231bfee83568: [web] Use `EnumSettingOption` components in `ThreadSettingsPrivacyTab` (authored by atul). · Explain WhyMay 19 2022, 10:55 AM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM231bfee83568: [web] Use `EnumSettingOption` components in `ThreadSettingsPrivacyTab`.

Revision Contents
Changeset List

PathSize
web/
modals/
threads/
settings/
6 lines
115 lines

Diff 12696

View Options

web/modals/threads/settings/thread-settings-privacy-tab.css

Loading...
View Options

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

Loading...