This differential updates input type of useChangeThreadSettings to accept union of thin and thick thread specific data.
Details
- Reviewers
tomek kamil - Commits
- rCOMM3909f7edd16f: Update input type of useChangeThreadSettings
test that changing various thread settings works for both thin and thick threads and results in the right notif.
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
native/chat/settings/thread-settings-description.react.js | ||
---|---|---|
267 ↗ | (On Diff #44031) | Why is it always false? |
lib/actions/thread-actions.js | ||
---|---|---|
124–128 ↗ | (On Diff #44031) | wondering, if this could be improved even more - for thick threads we're not able to modify thread members here (it's done via different spec), so we could remove the newMemberIDs field from UpdateThreadRequest |
native/chat/settings/thread-settings-description.react.js | ||
267 ↗ | (On Diff #44031) | it shouldn't - it's possbile for DMs to modify description, but it's probably bug from the previous code |
native/chat/settings/thread-settings-name.react.js | ||
12–16 ↗ | (On Diff #44031) | can be merged (applies to other files too) |
Made significant changes to address feedback regarding adding members to thick threads.
lib/actions/thread-actions.js | ||
---|---|---|
126 | Typo here |
web/modals/threads/settings/thread-settings-general-tab.react.js | ||
---|---|---|
23–24 | Why are we replacing this type by ThickThreadChanges? Are we still supporting thin threads? |
web/modals/threads/settings/thread-settings-general-tab.react.js | ||
---|---|---|
23–24 | This comment was not responded to |
web/modals/threads/settings/thread-settings-general-tab.react.js | ||
---|---|---|
23–24 | I am sorry for not documenting my answer here. However necessary changes were applied. ThickThreadChanges was renamed to ThreadChanges and ThreadChanges was renamed to ThinThreadChanges and used only in places where new members are added to thin thread. |
web/modals/threads/settings/thread-settings-general-tab.react.js | ||
---|---|---|
23–24 | Thanks for explaining! |