Campaign/applytheme center channel bg #7373

deanwhillier · 2021-01-22T00:02:54Z

Summary

ApplyTheme Campaign: This PR completes the merge of the theme.centerChannelBg commits from the campaign branch.

Dev approval for the changes was completed on the original PR(s), so we are just looking for rubber stamp approvals by dev's to satisfy merge requirements.

As QA was skipped on PR merges into campaign branches, this PR will need proper QA before merging. See below for details on what to QA for this PR.

Ticket Link

Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 620 mattermost#15887
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 621 mattermost#15888
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 622 mattermost#15889
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 627 mattermost#15890
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 629 mattermost#15891
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 631 mattermost#15892
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 633 mattermost#15893
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 635 mattermost#15894
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 636 mattermost#15895
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 640 mattermost#15896
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 641 mattermost#15897
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 668 mattermost#15898
Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 669 mattermost#15899

Related Pull Requests

See individual tickets for original PRs

Address PR feedback

…. 631 (#6733) * migrated call to changeCss for .app__body .popover.left>.arrow:after * fix: replace v with var in sass/components/_popover.scss

…. 633 (#6732) * migrated changeCss to css variable for .app__body .form-control * replace var with v in _forms.scss

Replace Javascript variable `theme.centerChannelBg` with CSS variable `--center-channel-bg`.

…. 635 (#6724)

…. 621 (#15888) (#6771) Remove the line as it is already taken into account unconditionally at line 623 thanks to '.app__body .post .Menu .MenuItem' selector When this was added in commit c8ae25a cited selector wasn't present

…. 629 (#6734) * migrated call to changeCss for .app__body .popover.bottom>.arrow:after * fix: replace v with var for .app__body .popover.bottom>.arrow:after Co-authored-by: Hossein Ahmadian-Yazdi <[email protected]> * fix: addressed review comments for #6734 Co-authored-by: Hossein Ahmadian-Yazdi <[email protected]>

…. 668 (#6741) * Update utils.jsx * Update _post.scss * Update utils.jsx * Update utils.jsx * Update sass/components/_post.scss Co-authored-by: Devin Binnie <[email protected]> Co-authored-by: Devin Binnie <[email protected]> Co-authored-by: Dean Whillier <[email protected]>

…. 641 (#6800) Signed-off-by: Baibhab Debnath <[email protected]>

) * change #channel*view.channel-view background to use css variables * change css selectors * change css selector to .channel-view

- was added back during merge conflict resolution

- technically not even used because of the `display: none` on the parent `.arrow` container, but fixing in case it’s used again in the future.

…b.com/d-schaffer/mattermost-webapp into campaign/applytheme_center-channel-bg

- variable should have transparency

deanwhillier · 2021-01-22T00:11:01Z

sass/base/_structure.scss

@@ -84,6 +84,7 @@ body {
 @include clearfix;
 height: 100%;
 position: relative;
+ background: var(--center-channel-bg);


@jgilliam17, this affects the background colour of the posts area. Interestingly there are about 5 layers on top with the same colour defined, so not sure it will ever come into play. 🤷‍♂️

That really sounds like our CSS 😛

deanwhillier · 2021-01-22T01:54:32Z

sass/components/_forms.scss

@@ -163,6 +163,7 @@
 .form-control {
 color: v(center-channel-color);
 border-color: rgba(var(--center-channel-color-rgb), 0.16);
+ background: v(center-channel-bg);


@jgilliam17, this one affects input field backgrounds

deanwhillier · 2021-01-22T04:28:56Z

sass/components/_modal.scss

@@ -723,7 +723,7 @@
 font-weight: 900;
 justify-content: center;
 align-items: center;
-
+ background: var(--center-channel-bg);


@jgilliam17, I honestly can't find this anywhere in actual use anymore. @devinbinnie, @hmhealey, how about you guys?

I found a reference to nav-tabs in QuickSwitchModal

See below comment.

deanwhillier · 2021-01-22T04:30:34Z

sass/components/_popover.scss

+ > .arrow {
+ &:after {
+ border-bottom-color: var(--center-channel-bg);
+ }
+ }
+ }
+
+ &.left {
+ > .arrow {
+ &:after {
+ border-left-color: var(--center-channel-bg);
+ }
+ }


@jgilliam17, these aren't used as the .arrow is permanently hidden with display: none; just a few lines up. I've left them in for posterity's sake.

deanwhillier · 2021-01-22T04:42:02Z

sass/components/_post.scss

+ &:hover {
+ color: var(--center-channel-bg);
+ }
+ background: var(--center-channel-bg);


@jgilliam17, for the 'Show More' button on long posts, the text colour on hover and the default background colour.

deanwhillier · 2021-01-22T04:51:37Z

sass/components/_scrollbar.scss

+ scrollbar-face-color: rgba(var(--center-channel-bg-rgb), .1);
 scrollbar-highlight-color: #7D7E94;
 scrollbar-shadow-color: #2D2C4D;
- scrollbar-track-color: rgba(0, 0, 0, .1);
+ scrollbar-track-color: rgba(var(--center-channel-bg-rgb), .1);


@jgilliam17, I think these only apply to Windows and possibly only IE, so may not be needed anymore. Probably best to check Edge and the other Windows browsers. They supposedly change scrollbar colours, possibly of the scrollbar for posts.

There's new tags for WebKit browsers at least: ::-webkit-scrollbar-*
I assume there are similar ones for Firefox

deanwhillier · 2021-01-22T05:11:52Z

sass/layout/_sidebar-menu.scss

+.app__body {
+ .modal-tabs {
+ .nav-tabs > li {
+ background-color: var(--center-channel-bg);
+ }
+ }
+}


@jgilliam17, ditto to the one above ... I honestly can't find this anywhere in actual use anymore. @devinbinnie, @hmhealey, how about you guys?

In the context of modal-tabs I can't find reference to it either. Would be worth making sure it's not used by QuickSwitchModal but otherwise we can remove.

I investigated the QuickSwitchModal. modal-tabs and nav-tabs don't seem to be used together here or anywhere else. Additionally, the tabs in the QuickSwitchModal are manually disabled (components/quick_switch_modal/index.tsx, ln. 14).

Just wanted to make sure I didn't miss anything 😛

deanwhillier · 2021-01-22T05:39:27Z

@jgilliam17, this other PR was merged directly to master and should have been a part of this PR. Might be a good idea to give it a quick once over as well even though it's been in the wild for some time now. It's the background colour of the heading when you search for channels using in: in the search bar. I'm not really sure what the purpose of it is though as the overall background is the same colour.

deanwhillier · 2021-01-26T18:35:09Z

/update-branch

jgilliam17

Thanks @deanwhillier
Tested, looks to merge.

Verified PR 6735 that has been already merged, background heading color when you search for channels using in: in the search bar - as expected.
Verified background color of the posts area, input field backgrounds
and ‘Show More' button on long posts, the text color on hover and the default background color.
Verified scroll bars on Windows, for both posts and sidebar.

mm-cloud-bot · 2021-01-27T19:00:10Z

Test server destroyed

d-schaffer and others added 18 commits October 9, 2020 00:37

Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 640

55a679b

Merge branch 'master' into campaign/applytheme_center-channel-bg

c6bc1ed

Apply suggestions from code review

d87f755

Address PR feedback

[GH-15892] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln…

83c6849

…. 631 (#6733) * migrated call to changeCss for .app__body .popover.left>.arrow:after * fix: replace v with var in sass/components/_popover.scss

[GH-15893] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln…

41aecf9

…. 633 (#6732) * migrated changeCss to css variable for .app__body .form-control * replace var with v in _forms.scss

Update scrollbar-track-color (#6725)

976cba5

Replace Javascript variable `theme.centerChannelBg` with CSS variable `--center-channel-bg`.

[GH-15894] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln…

853840c

…. 635 (#6724)

Merge branch 'master' into campaign/applytheme_center-channel-bg

c508d46

migrated changeCss() to CSS variable (#6714)

7d1c6f3

[MM-29510] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln…

ff77cda

…. 641 (#6800) Signed-off-by: Baibhab Debnath <[email protected]>

[MM-29500] Migrate changeCSS() to CSS variable in utils/utils.jsx (#6959

7068600

) * change #channel*view.channel-view background to use css variables * change css selectors * change css selector to .channel-view

re-remove changeCss entry

031a66e

- was added back during merge conflict resolution

original PR submission has a bug, fixing

60b171a

- technically not even used because of the `display: none` on the parent `.arrow` container, but fixing in case it’s used again in the future.

Merge branch 'campaign/applytheme_center-channel-bg' of https://githu…

fdcaeab

…b.com/d-schaffer/mattermost-webapp into campaign/applytheme_center-channel-bg

fix conversion bug

bce810d

- variable should have transparency

deanwhillier requested review from hmhealey, devinbinnie and jgilliam17 January 22, 2021 00:03

deanwhillier added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Jan 22, 2021

limit channel background colour to channel view

941d7dd

deanwhillier added the Work in Progress Not yet ready for review label Jan 22, 2021

deanwhillier commented Jan 22, 2021

View reviewed changes

deanwhillier removed the Work in Progress Not yet ready for review label Jan 22, 2021

devinbinnie approved these changes Jan 22, 2021

View reviewed changes

jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 22, 2021

hmhealey approved these changes Jan 26, 2021

View reviewed changes

hmhealey removed the 2: Dev Review Requires review by a core commiter label Jan 26, 2021

Merge branch 'master' into campaign/applytheme_center-channel-bg

30208d0

jgilliam17 approved these changes Jan 27, 2021

View reviewed changes

jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jan 27, 2021

deanwhillier merged commit dd56e65 into master Jan 27, 2021

deanwhillier deleted the campaign/applytheme_center-channel-bg branch January 27, 2021 19:24

amyblais added this to the v5.33 milestone Jan 27, 2021

amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Campaign/applytheme center channel bg #7373

Campaign/applytheme center channel bg #7373

deanwhillier commented Jan 22, 2021

deanwhillier Jan 22, 2021

hmhealey Jan 26, 2021

deanwhillier Jan 22, 2021

deanwhillier Jan 22, 2021

devinbinnie Jan 22, 2021

devinbinnie Jan 22, 2021

deanwhillier Jan 22, 2021 •

edited

Loading

deanwhillier Jan 22, 2021

deanwhillier Jan 22, 2021

devinbinnie Jan 22, 2021

deanwhillier Jan 22, 2021

devinbinnie Jan 22, 2021

deanwhillier Jan 22, 2021

deanwhillier commented Jan 22, 2021

deanwhillier commented Jan 26, 2021

jgilliam17 left a comment

mm-cloud-bot commented Jan 27, 2021

Campaign/applytheme center channel bg #7373

Campaign/applytheme center channel bg #7373

Conversation

deanwhillier commented Jan 22, 2021

Summary

Ticket Link

Related Pull Requests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deanwhillier Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deanwhillier commented Jan 22, 2021

deanwhillier commented Jan 26, 2021

jgilliam17 left a comment

Choose a reason for hiding this comment

mm-cloud-bot commented Jan 27, 2021

deanwhillier Jan 22, 2021 •

edited

Loading