Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix issue that caused the value of certain settings to be inverted #6896

Merged
merged 4 commits into from
Oct 7, 2021

Conversation

psrpinto
Copy link
Contributor

@psrpinto psrpinto commented Oct 1, 2021

Fixes element-hq/element-web#19201

When using the SettingsStore.watchSetting() method for settings which have an invertedSettingName, the newValueAt argument passed to the callback function, would erroneously contain the inverted value.

This was making it so that such settings appeared to be disabled when they should in fact be enabled, or vice-versa. This was however only the case for code which took in account the newValueAt argument. Code using the newValue argument was not affected.

The settings which have an invertedSettingName, and were thus potentially impacted are:

  • MessageComposerInput.dontSuggestEmoji
  • hideRedactions
  • hideJoinLeaves
  • hideAvatarChanges
  • hideDisplaynameChanges
  • hideReadReceipts
  • Pill.shouldHidePillAvatar
  • TextualBody.disableBigEmoji
  • dontSendTypingNotifications
  • TagPanel.disableTagPanel
  • webRtcForceTURN

This appears to be a regression introduced in #6261.


Here's what your changelog entry will look like:

🐛 Bug Fixes

Preview: https://615f03d07fd6f32726b13383--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

When using the SettingsStore.watchSetting() method for settings which have an
invertedSettingName, the newValueAt argument passed to the callback function,
would erroneously contain the inverted value.

This was making it so that such settings appeared to be disabled when they
should in fact be enabled, or vice-versa. This was however only the case for
code which took in account the newValueAt argument. Code using the newValue
argument was not affected.

The settings which have an invertedSettingName, and were thus potentially
impacted are:

- MessageComposerInput.dontSuggestEmoji
- hideRedactions
- hideJoinLeaves
- hideAvatarChanges
- hideDisplaynameChanges
- hideReadReceipts
- Pill.shouldHidePillAvatar
- TextualBody.disableBigEmoji
- dontSendTypingNotifications
- TagPanel.disableTagPanel
- webRtcForceTURN

Signed-off-by: Paulo Pinto <paulo.pinto@automattic.com>
@psrpinto psrpinto requested a review from a team as a code owner October 1, 2021 15:57
@SimonBrandner SimonBrandner added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Oct 1, 2021
Copy link
Contributor

@Palid Palid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the pull request comments, otherwise LGTM.

src/settings/SettingsStore.ts Outdated Show resolved Hide resolved
src/settings/SettingsStore.ts Outdated Show resolved Hide resolved
Signed-off-by: Paulo Pinto <paulo.pinto@automattic.com>
Signed-off-by: Paulo Pinto <paulo.pinto@automattic.com>
Signed-off-by: Paulo Pinto <paulo.pinto@automattic.com>
@psrpinto
Copy link
Contributor Author

psrpinto commented Oct 7, 2021

Agree with the remarks @Palid, thanks! I believe I've address them all, let me know if there's anything else you'd like me to change.

@Palid Palid enabled auto-merge (squash) October 7, 2021 14:42
@Palid Palid merged commit 1b334e4 into matrix-org:develop Oct 7, 2021
@novocaine
Copy link
Contributor

:shipit: thanks!

@psrpinto psrpinto deleted the fix/inverted-settings-toggle branch October 7, 2021 14:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabled "Show read receipts sent by other users" Toggle hides "seen by" column in timeline
4 participants