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

[MM-39346] - Make scrollbar visible in all themes #9215

Merged
merged 1 commit into from
Oct 22, 2021
Merged

Conversation

nevyangelova
Copy link
Contributor

Summary

This PR omits the 'ChangeCSS' application for the scrollbar background and uses a CSS variable instead, which solves the visibility issues.

Ticket Link

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

Release Note

NONE

@nevyangelova nevyangelova added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 20, 2021
@nevyangelova nevyangelova added this to the v6.1.0 milestone Oct 20, 2021
Copy link
Contributor

@michelengelen michelengelen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

I assume it's intentional that we're getting the scrollbar colour from a different theme field, right?

@nevyangelova
Copy link
Contributor Author

I assume it's intentional that we're getting the scrollbar colour from a different theme field, right?

correct. As of now there isn't a requirement for the scrollbar to have it's own field, instead, it should take the value that you see in the other small-ish elements around the sidebar like the channel navigator button.

@hmhealey hmhealey removed the 2: Dev Review Requires review by a core commiter label Oct 21, 2021
@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 22, 2021
Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thanks @nevyangelova
Tested, looks good to merge.

  • Verified scrollbar is visible in all themes.

@jgilliam17 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 Oct 22, 2021
@mm-cloud-bot
Copy link

Test server destroyed

@jgilliam17 jgilliam17 merged commit 832acae into master Oct 22, 2021
@jgilliam17 jgilliam17 deleted the theming-issues branch October 22, 2021 14:48
@amyblais
Copy link
Member

/cherry-pick cloud

@mattermod
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit to mattermost-build/mattermost-webapp that referenced this pull request Oct 22, 2021
Co-authored-by: Nevyana Angelova <[email protected]>
(cherry picked from commit 832acae)
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Oct 22, 2021
@mattermod mattermod added the CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone label Oct 22, 2021
mattermod added a commit to mattermost-build/mattermost-webapp that referenced this pull request Oct 22, 2021
amyblais pushed a commit that referenced this pull request Oct 22, 2021
Co-authored-by: Nevyana Angelova <[email protected]>
(cherry picked from commit 832acae)

Co-authored-by: Nev Angelova <[email protected]>
Co-authored-by: Mattermod <[email protected]>
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/Not Needed Does not require a changelog entry CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants