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

[MM-16541] Remove perfect-scrollbar package #4382

Merged
merged 3 commits into from
Dec 4, 2019
Merged

[MM-16541] Remove perfect-scrollbar package #4382

merged 3 commits into from
Dec 4, 2019

Conversation

fmunshi
Copy link
Contributor

@fmunshi fmunshi commented Nov 29, 2019

Summary

  • The original ticket was to "Update PerfectScrollbar to 1.4.0 and not use it with jQuery
  • However after working on that for a little while I realized that this library was not being used correctly and in my opinion was making the scrollbar worse than the regular browser scrollbar in some cases
  • Theres also the fact that this library is not even used in most modals / scrollbars just a select few (probably from older components)

The places that should be tested to ensure that the experience is still good enough are:

  • User account settings and access history modal
  • Popover list of channel members (top right)
  • Team settings modal
  • Channel notifications

Ticket Link

Fixes: mattermost/mattermost#11422
JIRA: https://mattermost.atlassian.net/browse/MM-16541

Screenshots

I have recorded two gifs:

  • The first gif (With PerfectScrollbar) has some strange behaviour with what seems to be conflicting styles (a thicker scrollbar container but the actual scroll wheel is still small)
  • The second gif (Without PerfectScrollbar) maintains the scrollbar that is present everywhere else in the app

With PerfectScrollbar
Without PerfectScrollbar

@hanzei hanzei added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Nov 30, 2019
saturninoabril
saturninoabril approved these changes Dec 4, 2019
View reviewed changes
Copy link
Member

@saturninoabril saturninoabril left a comment

Choose a reason for hiding this comment

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

Tested and looks good to me.

@saturninoabril saturninoabril added QA Review Done and removed 3: QA Review Requires review by a QA tester labels Dec 4, 2019
jespino
jespino approved these changes Dec 4, 2019
View reviewed changes
Copy link
Member

@jespino jespino 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. Thanks @fm2munsh! 🎉

@jespino jespino requested review from asaadmahmood and removed request for asaadmahmood December 4, 2019 11:35
asaadmahmood
asaadmahmood approved these changes Dec 4, 2019
View reviewed changes
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.

Looks good. We just can't style the default scrollbar is specific browsers like IE, but I think it's fine.

@hmhealey
Copy link
Member

hmhealey commented Dec 4, 2019

@asaadmahmood IE isn't a problem any more since we don't have to support it any more 😁

hmhealey
hmhealey approved these changes Dec 4, 2019
View reviewed changes
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.

Thanks for the PR! Many of us have been frustrated with perfect-scrollbar for a long time, so it's awesome to have it finally removed

@asaadmahmood
Copy link
Contributor

@hmhealey Yup, and we can also just use the react native scrollbars if we want a replacement.

@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Dec 4, 2019
@hmhealey hmhealey added this to the v5.20.0 milestone Dec 4, 2019
@hmhealey hmhealey merged commit 2bae672 into mattermost:master Dec 4, 2019
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jan 17, 2020
@fmunshi fmunshi deleted the MM-16541-remove-perfect-scrollbar branch January 29, 2020 04:32
deanwhillier pushed a commit that referenced this pull request Oct 14, 2020
@KuSh
Migrate changeCSS(sideText) to CSS variable --sidebartext in utils/ut…
b6066c0 
…ils.jsx (#6697)

* [MM-29389] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 510 (#15828)

This line was unconditionally replaced by line 824 using --link-color so I directly used --link-color in CSS

Fixes mattermost/mattermost#15828
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29389

* [MM-29390] Remove references to removed perfect-scrollbar component (#15829)

While analyzing mattermost/mattermost#15829 I couldn't found where ps-container class was used
It turns out it was an old dependency removed by PR #4382
This commit removes remaining references

Fixes mattermost/mattermost#15829
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29390
deanwhillier added a commit that referenced this pull request Oct 16, 2020
@deanwhillier @KuSh @mattermod
Campaign/applytheme sidebar text (#6775)
4e05760 
* Remove an unused changeCSS() style (#6680)

Fixes mattermost/mattermost#15837
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29399

* [MM-29392] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 516 (#15831) (#6695)

Fixes mattermost/mattermost#15831
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29392

* [MM-29397] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln 525 (#15836) (#6686)

Remove the line as style is already applied unconditionally and more broadly by changeCss() call at line 515

Fixes mattermost/mattermost#15836
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29397

* [MM-29396] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 523 (#15835) (#6692)

Remove the line as it isn't possible that .sidebar__switcher button contains a svg

Fixes mattermost/mattermost#15835
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29396

* [MM-29391] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 513 (#15830) (#6696)

Fixes mattermost/mattermost#15830
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29391

* [MM-29393] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 519 (#15832) (#6694)

Fixes mattermost/mattermost#15832
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29393

* [MM-29394] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 520 (#15833) (#6693)

Fixes mattermost/mattermost#15833
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29394

Co-authored-by: Dean Whillier <[email protected]>

* Migrate changeCSS(sideText) to CSS variable --sidebartext in utils/utils.jsx (#6697)

* [MM-29389] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 510 (#15828)

This line was unconditionally replaced by line 824 using --link-color so I directly used --link-color in CSS

Fixes mattermost/mattermost#15828
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29389

* [MM-29390] Remove references to removed perfect-scrollbar component (#15829)

While analyzing mattermost/mattermost#15829 I couldn't found where ps-container class was used
It turns out it was an old dependency removed by PR #4382
This commit removes remaining references

Fixes mattermost/mattermost#15829
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29390

Co-authored-by: Nicolas Le Cam <[email protected]>
Co-authored-by: Mattermod <[email protected]>
jfrerich pushed a commit that referenced this pull request Oct 23, 2020
@deanwhillier @KuSh
@mattermod @jfrerich
Campaign/applytheme sidebar text (#6775)
c9a16e0 
* Remove an unused changeCSS() style (#6680)

Fixes mattermost/mattermost#15837
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29399

* [MM-29392] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 516 (#15831) (#6695)

Fixes mattermost/mattermost#15831
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29392

* [MM-29397] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln 525 (#15836) (#6686)

Remove the line as style is already applied unconditionally and more broadly by changeCss() call at line 515

Fixes mattermost/mattermost#15836
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29397

* [MM-29396] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 523 (#15835) (#6692)

Remove the line as it isn't possible that .sidebar__switcher button contains a svg

Fixes mattermost/mattermost#15835
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29396

* [MM-29391] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 513 (#15830) (#6696)

Fixes mattermost/mattermost#15830
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29391

* [MM-29393] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 519 (#15832) (#6694)

Fixes mattermost/mattermost#15832
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29393

* [MM-29394] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 520 (#15833) (#6693)

Fixes mattermost/mattermost#15833
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29394

Co-authored-by: Dean Whillier <[email protected]>

* Migrate changeCSS(sideText) to CSS variable --sidebartext in utils/utils.jsx (#6697)

* [MM-29389] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 510 (#15828)

This line was unconditionally replaced by line 824 using --link-color so I directly used --link-color in CSS

Fixes mattermost/mattermost#15828
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29389

* [MM-29390] Remove references to removed perfect-scrollbar component (#15829)

While analyzing mattermost/mattermost#15829 I couldn't found where ps-container class was used
It turns out it was an old dependency removed by PR #4382
This commit removes remaining references

Fixes mattermost/mattermost#15829
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29390

Co-authored-by: Nicolas Le Cam <[email protected]>
Co-authored-by: Mattermod <[email protected]>
calebroseland pushed a commit that referenced this pull request Oct 27, 2020
@deanwhillier @KuSh
@mattermod @calebroseland
Campaign/applytheme sidebar text (#6775)
dd4dae4 
* Remove an unused changeCSS() style (#6680)

Fixes mattermost/mattermost#15837
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29399

* [MM-29392] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 516 (#15831) (#6695)

Fixes mattermost/mattermost#15831
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29392

* [MM-29397] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln 525 (#15836) (#6686)

Remove the line as style is already applied unconditionally and more broadly by changeCss() call at line 515

Fixes mattermost/mattermost#15836
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29397

* [MM-29396] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 523 (#15835) (#6692)

Remove the line as it isn't possible that .sidebar__switcher button contains a svg

Fixes mattermost/mattermost#15835
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29396

* [MM-29391] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 513 (#15830) (#6696)

Fixes mattermost/mattermost#15830
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29391

* [MM-29393] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 519 (#15832) (#6694)

Fixes mattermost/mattermost#15832
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29393

* [MM-29394] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 520 (#15833) (#6693)

Fixes mattermost/mattermost#15833
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29394

Co-authored-by: Dean Whillier <[email protected]>

* Migrate changeCSS(sideText) to CSS variable --sidebartext in utils/utils.jsx (#6697)

* [MM-29389] Migrate changeCSS() to CSS variable in utils/utils.jsx, ln. 510 (#15828)

This line was unconditionally replaced by line 824 using --link-color so I directly used --link-color in CSS

Fixes mattermost/mattermost#15828
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29389

* [MM-29390] Remove references to removed perfect-scrollbar component (#15829)

While analyzing mattermost/mattermost#15829 I couldn't found where ps-container class was used
It turns out it was an old dependency removed by PR #4382
This commit removes remaining references

Fixes mattermost/mattermost#15829
JIRA ticket: https://mattermost.atlassian.net/browse/MM-29390

Co-authored-by: Nicolas Le Cam <[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.
Reviewers

@jespino jespino jespino approved these changes

@hmhealey hmhealey hmhealey approved these changes

@saturninoabril saturninoabril saturninoabril approved these changes

@asaadmahmood asaadmahmood asaadmahmood approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation QA Review Done
Projects
None yet
Milestone
v5.20.0
7 participants
@fmunshi @hmhealey @asaadmahmood @jespino @saturninoabril @hanzei @amyblais