Mattermost-server: Remove usage of jQuery from root components

Created on 31 Jul 2020  路  12Comments  路  Source: mattermost/mattermost-server

We've been working for some time to remove jQuery as a dependency of the Mattermost web app to reduce the size of the JS bundle in the web app and to simplify our code by using equivalent functions that are usable on all browsers that we support.

As part of this, we should remove the usage of jQuery from the following files in the web app:

  • components/logged_in/logged_in.jsx
  • components/root/root.jsx
  • components/tutorial/tutorial_view.jsx

In addition to removing the usage of jQuery, E2E tests using Cypress should be added to ensure that:

  • The hover effects around posts wrap smoothly around date separators in the post list and RHS thread view
  • Pressing the backspace key without an input focused should not send the browser back in history
  • With Mattermost open in multiple tabs, logging into one tab should trigger login on other tabs
  • With Mattermost open in multiple tabs, logging out of one tab should trigger logout on other tabs
  • The app is correctly themed even with the tutorial open

If you have any questions, feel free to ask @hmhealey (or @harrison on our 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: https://mattermost.atlassian.net/browse/MM-27448

Medium Hacktoberfest Help Wanted PR Exists TecReactJS

All 12 comments

@hmhealey I'd like to work on this too.

Same as the other one, let me know if you have any questions 馃檪

  • With Mattermost open in multiple tabs, logging into one tab should trigger login on other tabs

@hmhealey I want to point out that this馃憜馃徑 doesn't currently work as described unless the user reloads the other tabs as well.

Perhaps that is a regression that was introduced somewhere along the way.

I've tried debugging to see why. It appears the signalLogin() (https://github.com/mattermost/mattermost-webapp/blob/061b158d471ef99b3ae74279e7f36050d3cce761/stores/browser_store.jsx#L56) method is never called anywhere.

@hmhealey What do you think?

Good find. Maybe just migrate the code, but don't bother adding any tests for that part. I'll file a ticket to look into if we need to re-add that or if we can just remove it.

I could've sworn that that was tested as part of our manual tests, but maybe it isn't since I can't find any evidence of that being called as far back as 4.3 (the earliest release branch that we still have around). Maybe it wouldn't have been randomly broken if we did have e2e tests that far back 馃槢

  • The hover effects around posts wrap smoothly around date separators in the post list and RHS thread view

Hi, @hmhealey I'm a bit confused about this, I don't know exactly what it means. Is there a demo (e.g a gif) or something that I can look at to get an idea of exactly how this currently works so I know what tests to write?

I don't have anything set up to record a gif right now, but hopefully I can explain it well enough. When you hover over a post and the background of the post becomes darker (or lighter when a dark theme), the effect wraps around the text of the New Messages line and the Date lines on the post list like this:
Screen Shot 2020-08-27 at 4 52 30 PM
Screen Shot 2020-08-27 at 4 52 28 PM
Screen Shot 2020-08-27 at 4 52 46 PM
Screen Shot 2020-08-27 at 4 52 44 PM
Notice how there's a rounded rectangle around the text that doesn't become darker/lighter?

This code is responsible for it currently.

I'm not entirely sure if we'll be able to add an automated test for this since Cypress doesn't have great support for hovering on something with a mouse, but I think we might be able to test this by using Cypress's trigger method to simulate the mouse events and ensure they set the right classes on those components.

This code is responsible for it currently.

I also figured that block of code is responsible for that functionality. So naturally, I commented it out to see if it was, but I noticed it didn't have any effect. I think that block of code doesn't have any effect and the current wrapping functionality is actually achieved using CSS and the hover pseudo class, rather than the class names added or removed on hover.

I'm still debugging this to figure out which code gives this effect, but I just wanted to quickly drop this here.

That's great to find out. That code has always been ugly, so it'll be good to get rid of it. We don't even use the date-separator class that's referenced by part of that code since the newer component uses a DateSeparator class.

Looking into it a bit more, the background of the post actually extends all the way to the divider lines now which wasn't the case back when this code was actually needed. The PostListVirtualized component is responsible for telling the post when it needs extra padding to account for those lines, and then it uses hover pseudo classes as you mentioned.

Thanks again for your help understanding what is and isn't needed of this old code. 馃檪

Hi @CEOehis. I just wanted to check in and see if you're still working on this, or if you need any help with anything on it?

Hi @hmhealey I'm still working on this. I'll turn it in in a bit.

Was this page helpful?
0 / 5 - 0 ratings