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

[MM-32087] - Add theme variable for sidebar team bar #7389

Merged
merged 20 commits into from
Feb 17, 2021
Merged

[MM-32087] - Add theme variable for sidebar team bar #7389

merged 20 commits into from
Feb 17, 2021

Conversation

nevyangelova
Copy link
Contributor

Summary

This PR adds a new theme variable for team sidebar text which would replace the currently constructed colour of the team bar, inclusive of channelHeaderColour + opacity.

Ticket Link

https://mattermost.atlassian.net/browse/MM-32087

Related Pull Requests

mattermost/mattermost-redux#1342

@nevyangelova nevyangelova added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jan 25, 2021
@nevyangelova nevyangelova added this to the v5.32.0 milestone Jan 25, 2021
@amyblais amyblais removed this from the v5.32.0 milestone Jan 25, 2021
@nevyangelova nevyangelova added the 1: UX Review Requires review by a UX Designer label Jan 25, 2021
devinbinnie
devinbinnie reviewed Jan 25, 2021
View reviewed changes
Copy link
Member

@devinbinnie devinbinnie left a comment

Choose a reason for hiding this comment

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

Since this is a new theme variable and many current custom themes that users (like myself) use won't have this new variable. What's the plan for those themes wrt. migration to using this new variable?

Are we able to fall back on a different value if need be?

@nevyangelova
Copy link
Contributor Author

Since this is a new theme variable and many current custom themes that users (like myself) use won't have this new variable. What's the plan for those themes wrt. migration to using this new variable?

Are we able to fall back on a different value if need be?

I was thinking a migration to use the new variable. Would it be too tedious?

@devinbinnie
Copy link
Member

Since this is a new theme variable and many current custom themes that users (like myself) use won't have this new variable. What's the plan for those themes wrt. migration to using this new variable?
Are we able to fall back on a different value if need be?

I was thinking a migration to use the new variable. Would it be too tedious?

So like a database migration script? That would be alright with me.
Another possible solution would be to null check and fall back on an existing variable.

matthewbirtch
matthewbirtch reviewed Jan 25, 2021
View reviewed changes
components/widgets/team_icon/team_icon.scss Outdated
@@ -47,41 +47,19 @@

&:hover {
box-shadow: 0 0 0 3px rgba(255, 255, 255, 0.32);
border-color: var(--sidebar-header-bg);
border-color: var(--sidebar-teambar-color);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we change to --sidebar-teambar-bg for clarity and consistency?

@matthewbirtch matthewbirtch added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 25, 2021
@matthewbirtch
Copy link
Contributor

@nevyangelova the new variable will need to be added in to the customer them UI in the Account Settings as well. Suggest we call it "Team Sidebar BG" in this context:

image

@nevyangelova nevyangelova mentioned this pull request Jan 26, 2021
Merged
@nevyangelova
Copy link
Contributor Author

Since this is a new theme variable and many current custom themes that users (like myself) use won't have this new variable. What's the plan for those themes wrt. migration to using this new variable?
Are we able to fall back on a different value if need be?

I was thinking a migration to use the new variable. Would it be too tedious?

So like a database migration script? That would be alright with me.
Another possible solution would be to null check and fall back on an existing variable.

Seems like a good discussion to have in triage today.

@nevyangelova
Copy link
Contributor Author

@matthewbirtch nice catch! Fixed.

matthewbirtch
matthewbirtch approved these changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@matthewbirtch matthewbirtch left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@jgilliam17
Copy link
Contributor

jgilliam17 commented Feb 2, 2021 •
edited
Loading

Thanks @nevyangelova
Seeing one small issue, when user selects a custom BG color for the team sidebar, it is not respected on drag. See video.
Edited. Actually, it seems to only impact team icons with an uploaded image.

Screen.Recording.2021-02-02.at.3.53.06.PM.mov

@nevyangelova
Copy link
Contributor Author

/update-branch

@hmhealey hmhealey added Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) 4: Reviews Complete All reviewers have approved the pull request and removed 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core commiter labels Feb 12, 2021
@nevyangelova
Copy link
Contributor Author

/update-branch

@nevyangelova nevyangelova removed the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Feb 17, 2021
@nevyangelova nevyangelova merged commit 88a356b into master Feb 17, 2021
@nevyangelova nevyangelova deleted the MM-32087 branch February 17, 2021 09:00
@mattermod mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Feb 17, 2021
@mm-cloud-bot
Copy link

Test server destroyed

1 similar comment
@mm-cloud-bot
Copy link

Test server destroyed

chetanyakan added a commit to brightscout-alpha/mattermost-webapp that referenced this pull request Feb 17, 2021
@chetanyakan
Merge branch 'master' of github.com:mattermost/mattermost-webapp into…
196cd1e 
… custom-status

* 'master' of github.com:mattermost/mattermost-webapp:
  [MM-32087] - Add theme variable for sidebar team bar (mattermost#7389)
  MM-32825 - suggestion box hidden by rhs (mattermost#7523)
  MM-32928 Remove custom-protocol-detection (mattermost#7532)
  [MM-32684] Fix RHS Autocomplete options cover the input box for some reply threads (mattermost#7504)
  MM-3190 show user limit modal when cloud limit exceed (mattermost#7496)
  MM-28736 Update dependencies (mattermost#7512)
  MM-31501 - Add upgrade-now link for admins in invite users (mattermost#7446)
chetanyakan added a commit to brightscout-alpha/mattermost-webapp that referenced this pull request Feb 17, 2021
@chetanyakan
Merge branch 'master' of github.com:mattermost/mattermost-webapp into…
25ce4bf 
… custom-status

* 'master' of github.com:mattermost/mattermost-webapp:
  MM-27400: tooltip for long channel names (mattermost#7527)
  Remove output from pkill (mattermost#7536)
  MM-27241 Private channel prompt for sysadmin (mattermost#7489)
  [MM-32087] - Add theme variable for sidebar team bar (mattermost#7389)
  MM-32825 - suggestion box hidden by rhs (mattermost#7523)
  MM-32928 Remove custom-protocol-detection (mattermost#7532)
  [MM-32684] Fix RHS Autocomplete options cover the input box for some reply threads (mattermost#7504)
  MM-3190 show user limit modal when cloud limit exceed (mattermost#7496)
  MM-28736 Update dependencies (mattermost#7512)
  MM-31501 - Add upgrade-now link for admins in invite users (mattermost#7446)
@amyblais amyblais added this to the v5.34 milestone Feb 26, 2021
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Mar 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matthewbirtch matthewbirtch matthewbirtch approved these changes

@hmhealey hmhealey hmhealey approved these changes

@devinbinnie devinbinnie devinbinnie approved these changes

@jgilliam17 jgilliam17 jgilliam17 approved these changes

Assignees

@nevyangelova nevyangelova

Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation QA Review Done
Projects
None yet
Milestone
v5.34.0
8 participants
@nevyangelova @devinbinnie @matthewbirtch @jgilliam17 @mm-cloud-bot @hmhealey @mattermod @amyblais