Page MenuHomePhabricator
Differential D3826

[web] Remove duplicate `thread[Name/Description]Disabled` props in `ThreadSettingsGeneralTab` component
ClosedPublic
Actions

Authored by atul on Apr 24 2022, 1:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 25, 3:44 AM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:39 PM
Unknown Object (File)
Thu, Dec 19, 1:36 PM
View All Files
Subscribers
• abosh
• adrian
ashoat
• karol

Details

Reviewers
• jacek
tomek
ashoat
Commits
rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in…
Summary

These props both get inputDisabled passed in from the parent component, so they can be "flattened" into a single prop.

Depends on D3825

Test Plan
  1. Open ThreadSettingsModal
  2. Make sure that the "General" tab continues to look/work as expected

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Apr 24 2022, 1:05 PM
Herald added subscribers: • abosh, • karol, • adrian, ashoat. · View Herald TranscriptApr 24 2022, 1:05 PM
Harbormaster completed remote builds in B8486: Diff 11831.Apr 24 2022, 1:10 PM
atul requested review of this revision.Apr 24 2022, 1:10 PM
atul added a child revision: D3827: [web] Push `onChange[Name/Description/Color]` callbacks down to `ThreadSettingsGeneralTab` component.Apr 24 2022, 1:49 PM
tomek accepted this revision.Apr 26 2022, 8:14 AM
tomek added a reviewer: ashoat.

Just wondering: are we planning to have threads with one input enabled and another disabled? It feels like changing a description is less destructive than changing a name and more users could be allowed to do that. On the other hand, reintroducing this option shouldn't be hard. Adding @ashoat to get his opinion.

atul updated this revision to Diff 11946.Apr 26 2022, 10:25 AM

rebase

atul added a comment.Apr 26 2022, 10:30 AM
In D3826#106968, @palys-swm wrote:

Just wondering: are we planning to have threads with one input enabled and another disabled? It feels like changing a description is less destructive than changing a name and more users could be allowed to do that. On the other hand, reintroducing this option shouldn't be hard. Adding @ashoat to get his opinion.

That's a good point, I could definitely see us having more fine-grained thread setting permissions in the future.

For now just maintaining the existing behavior, which I think should be fine for a refactoring stack of diffs.

Harbormaster completed remote builds in B8567: Diff 11946.Apr 26 2022, 10:32 AM
ashoat accepted this revision.Apr 27 2022, 11:43 AM

We can refactor later if we need to

This revision is now accepted and ready to land.Apr 27 2022, 11:43 AM
atul updated this revision to Diff 12070.Apr 28 2022, 1:22 PM

rebase before landing

Harbormaster completed remote builds in B8669: Diff 12070.Apr 28 2022, 1:27 PM
atul updated this revision to Diff 12073.Apr 28 2022, 1:31 PM

rebase before landing, actually

This revision was landed with ongoing or failed builds.Apr 28 2022, 1:33 PM
Closed by commit rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in… (authored by atul). · Explain Why
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMM024a3be9ffb0: [web] Remove duplicate `thread[Name/Description]Disabled` props in….
Harbormaster failed remote builds in B8672: Diff 12073!Apr 28 2022, 1:36 PM

Revision Contents
Changeset List

Diff 12074

View Options

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

Loading...
View Options

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

Loading...