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

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

Merged
merged 3 commits into from
Nov 25, 2020
Merged

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

merged 3 commits into from
Nov 25, 2020

Conversation

oliverJurgen
Copy link
Contributor

@oliverJurgen oliverJurgen commented Oct 29, 2020

Summary

Migrate changeCss('.app*_body #channel*view.channel-view', `background: ${theme.centerChannelBg}`) in line 620 to CSS variables. Css applied in sass/base/_structure.scss.

Ticket Link

Fixes mattermost/mattermost#15887

Jira: https://mattermost.atlassian.net/browse/MM-29500.

If I have to make changes in the PR, please do tell me. Thanks

@mattermod
Copy link
Contributor

Hello @oliverJurgen,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Copy link
Contributor

@asaadmahmood asaadmahmood left a comment

Choose a reason for hiding this comment

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

Please use it like this:

.channel-view {
        &#channel-view {
            background: var(--center-channel-bg);
        }
    }

Also please see if we need so much specificity or just .channel_view works.

Copy link
Member

@calebroseland calebroseland left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @oliverJurgen! Would you mind moving background to the plain .channel-view towards the bottom?

@calebroseland calebroseland self-requested a review October 30, 2020 20:24
@oliverJurgen
Copy link
Contributor Author

I applied the changes

@oliverJurgen
Copy link
Contributor Author

@calebroseland Hello, any more changes I should do?

@calebroseland
Copy link
Member

calebroseland commented Nov 5, 2020

@oliverJurgen I think it would be a good chance to simplify and put in just .channel-view down by :80. If you wouldn't mind, that'd be great :)

@oliverJurgen
Copy link
Contributor Author

sounds good, will do that :)

@oliverJurgen
Copy link
Contributor Author

please check my latest change @calebroseland :)

Copy link
Member

@calebroseland calebroseland left a comment

Choose a reason for hiding this comment

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

Thanks, @oliverJurgen!

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

@jasonblais
Copy link
Contributor

@calebroseland @asaadmahmood is this ready to merge?

@calebroseland
Copy link
Member

@jasonblais yes, my bad.

@calebroseland calebroseland merged commit 10e0897 into mattermost:campaign/applytheme_center-channel-bg Nov 25, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation and removed 2: Dev Review Requires review by a core commiter labels Nov 25, 2020
deanwhillier pushed a commit that referenced this pull request Jan 21, 2021
)

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

* change css selectors

* change css selector to .channel-view
deanwhillier added a commit that referenced this pull request Jan 27, 2021
* Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 640

* Apply suggestions from code review

Address PR feedback

* [GH-15892] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 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. 633 (#6732)

* migrated changeCss to css variable for .app__body .form-control

* replace var with v in _forms.scss

* Update scrollbar-track-color (#6725)

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

* [GH-15894] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 635 (#6724)

* [MM-29501] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 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

* migrated changeCss() to CSS variable (#6714)

* [GH-15891] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 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]>

* [GH-15898] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 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]>

* [MM-29510] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 641 (#6800)

Signed-off-by: Baibhab Debnath <[email protected]>

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

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

* change css selectors

* change css selector to .channel-view

* re-remove changeCss entry

- was added back during merge conflict resolution

* original PR submission has a bug, fixing

- 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.

* fix conversion bug

- variable should have transparency

* limit channel background colour to channel view

Co-authored-by: d-schaffer <[email protected]>
Co-authored-by: Mattermod <[email protected]>
Co-authored-by: Sudipto Ghosh <[email protected]>
Co-authored-by: Tony Ho <[email protected]>
Co-authored-by: Nicolas Le Cam <[email protected]>
Co-authored-by: Tobias Weichart <[email protected]>
Co-authored-by: Hossein Ahmadian-Yazdi <[email protected]>
Co-authored-by: AO <[email protected]>
Co-authored-by: Devin Binnie <[email protected]>
Co-authored-by: Baibhab Debnath <[email protected]>
Co-authored-by: oliverJurgen <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation Hacktoberfest hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants