Mattermost-server: Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 701

Created on 19 Oct 2020 · 2Comments · Source: mattermost/mattermost-server

Context:

We are working on replacing our older custom theme implementation (applyTheme()) currently used to change component colors throughout the webapp to match the selected theme. We have decided to utilize CSS variables as a replacement and so we now need to replace all occurances of changeCSS() within the applyTheme() function with corresponding CSS variables directly in the CSS files. This ticket is part of a larger campaign.

Migration TL;DR

In the Mattermost Webapp repo, go to line number 701 in the file utils/utils.jsx (link). You should see the following (might not be the exact line number):

changeCss('.app__body .channel-header .pinned-posts-button svg', 'fill:' <ins> changeOpacity(theme.centerChannelColor, 0.6));

Work off the branch campaign/applytheme_center-channel-color
The indicated line number may not be accurate but should be close
Replace the Javascript variable theme.centerChannelColor with the CSS variable --center-channel-color from the line above
Create a PR against the branch campaign/applytheme_center-channel-color when ready (not 'Master')

Additional details on how to migrate

For this ticket, we will be replacing the use of the Javascript variable theme.centerChannelColor from the above line of code with the CSS variable --center-channel-color. Please work off the branch campaign/applytheme_center-channel-color to avoid conflicts with other tickets for the same campaign. The line number may not be the same as when this ticket was created, but should be close.

You can use these steps to help determine where to apply the new CSS variable followed by removing the call to changeCSS() from utils/utils.jsx:

Make note of the CSS statement from the first argument to changeCSS and specifically the final selector (eg. .app__body .sidebar--menu)
Using your browsers dev tools, locate the element targeted by this CSS statement in the webapp
With the element selected in your dev tools, use the style browser to get an idea of how styles are currently applied to this element. This should help you track down where in the source code you will need to add the appropriate CSS variable in order to replace the style identified in the second argument to changeCSS() above (eg. 'background:' </ins> theme.sidebarBg)
Locate the appropriate file(s) within the source code where edits will need to be made (eg. sass/layout/_sidebar-menu.scss)
Make the necessary changes/additions to the CSS to apply the CSS variable and then remove the original JS line (shown above) from utils/utils.jsx.
Create a pull request with your changes against the branch campaign/applytheme_center-channel-color. We will be using this branch to review and QA changes in batches instead of on each individual ticket in the campaign given the number of overall tickets for this campaign.

Notes:

Some instances of changeCSS() affect the styles of sections of the UI that might not be visible by default. Remember to check modals, pop-ups, menus and different interaction states (hover, active, etc.) as necessary based on the particular line you are working on
Some instances of changeCSS() include the use of a custom changeOpacity() function when applying colours that need to be partially transparent. These instances will need to be replaced with the rgba CSS function. For example, changeOpacity(theme.sidebarBg, 0.3)) in utils/utils.jsx would become rgba(var(--sidebar-bg-rgb), 0.3) in CSS. Appending -rgb to the CSS variable name is necessary when using one of the theme variables to work with the rgba CSS function.
Some UI elements in the desktop web view will actually be different elements in the mobile web view, eg. the desktop and mobile primary/hamburger menus have completely separate markup
Please be sure to test the primary responsive breakpoint widths after migration (Desktop: 1920px, Tablet: 960px, Mobile: 768px)
Please be sure to test changing themes after migration to ensure the new CSS variable is being applied correctly. Switching back and forth between light and dark themes would be a good way to help determine this. Responsive breakpoints should also be tested with different themes.
It is possible that an existing changeCSS() line is no longer valid, in which case, just removing the line as part of your PR is a perfectly valid solution.

Example of migrated code

PR #6655

This PR includes several commits that each address a slightly different CSS statement in order to provide several examples of differing situations and how they can be migrated. We suggest taking a look at each to get an idea of possible differences you could run across and how the resulting migration was accomplished.

Questions

Feel free to ask for help by messaging either @deanwhillier or @hmhealey or by posting in the ~webapp channel on the Mattermost Community server.

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

JIRA ticket: https://mattermost.atlassian.net/browse/MM-29843

AreTechnical Debt Easy Help Wanted PR Exists TecReactJS

Source