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

[MM-10430] Fix behavior of ExperimentalTownSquareIsReadOnly and add ExperimentalHideTownSquareinLHS feature #1194

Merged
merged 5 commits into from
May 24, 2018

Conversation

saturninoabril
Copy link
Member

@saturninoabril saturninoabril commented May 9, 2018

Summary

Fix behavior of ExperimentalTownSquareIsReadOnly and add ExperimentalHideTownSquareinLHS feature

  • If ExperimentalTownSquareIsReadOnly is set to "true", do not automatically hide Town Square channel when it has no unread messages.
  • if ExperimentalHideTownSquareinLHS is set to "true", Town Square channel will be hidden when there are no unread messages. The channel should still be accessible via CTRL/CMD+K and the "More Channels" menus.

(Note: Will update commit of this PR once server and redux changes are merged)

@jasonblais @esethna @amyblais I'd like to request for your quick check once the server and redux PRs are merged - just to make sure that all are covered. Thanks!

Ticket Link

Jira ticket: [MM-10430]

Checklist

@saturninoabril saturninoabril added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) labels May 9, 2018
@saturninoabril saturninoabril added Work in Progress Not yet ready for review and removed 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter labels May 11, 2018
@lieut-data lieut-data removed their request for review May 18, 2018 13:55
@lieut-data
Copy link
Member

@saturninoabril, I've removed myself from the reviewers just to clear my /pulls/review-requested until you lift the Work in Progress tag -- let me know when you want me to take a look :)

@saturninoabril saturninoabril added 2: Dev Review Requires review by a core commiter and removed Work in Progress Not yet ready for review labels May 23, 2018
@saturninoabril
Copy link
Member Author

@lieut-data Sure and thanks! This should be good for dev review now.

!isFavoriteChannel(state.entities.preferences.myPreferences, channel.id);
let shouldHideChannel = false;
if (
Constants.DEFAULT_CHANNEL &&
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little unclear as to what this is checking -- surely Constants.DEFAULT_CHANNEL is always truthy? Was this meant to be a check against the channel name?

Copy link
Member Author

Choose a reason for hiding this comment

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

My mistake. It's check against the channel name. Though the shouldHideDefaultChannel has already that check, it immediately end other conditional checks to non-default channels.
PR updated.

@saturninoabril saturninoabril added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels May 24, 2018
@saturninoabril saturninoabril merged commit 2980bb2 into mattermost:master May 24, 2018
@saturninoabril saturninoabril deleted the MM-10430 branch May 24, 2018 21:20
@amyblais amyblais added the Changelog/Done Required changelog entry has been written label May 30, 2018
@amyblais amyblais added the Docs/Not Needed Does not require documentation label May 30, 2018
@lindalumitchell lindalumitchell added the Tests/Done Release tests have been written label Jun 13, 2018
hmhealey pushed a commit that referenced this pull request Aug 28, 2020
* patch upgrades

* fix broken webapp ci builds

* correct bot method types
hmhealey pushed a commit that referenced this pull request Mar 17, 2021
* patch upgrades

* fix broken webapp ci builds

* correct bot method types
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) Docs/Not Needed Does not require documentation Tests/Done Release tests have been written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants