MM-47663 : Test out MUI for menu components POC #11521

M-ZubairAhmed · 2022-11-03T13:10:55Z

Summary

Since material-ui has peer dependency on react-17, the base commits have been rebased from react 17 upgrade pr. Actual work on the pr starts from these commits

Ticket Link

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

Related Pull Requests

Has changes from #10836

Screenshots

Release Note

hmhealey · 2022-11-11T19:29:31Z

@M-ZubairAhmed Could you rebase this branch to sort out the conflicts and duplicate commits? I think that'd probably be the easiest way to fix at least the duplicated commits

…M-47663-b

michelengelen

just a few comments from my side ... looking very solid so far! Nice work!

michelengelen · 2022-11-23T14:01:30Z

components/at_mention/at_mention.tsx

+        if (e.stopPropagation) {
+            e.stopPropagation();
+        }


Suggested change

if (e.stopPropagation) {

e.stopPropagation();

}

e.stopPropagation?.();

I'm actually confused about why we need this here at all since e.stopPropagation should always be defined based on the type definition. Is the type definition perhaps incorrect?

this is not part of my pr. But the base react 17 pr.

I was confused as well. especially when using the built in React change handler type definitions this is typically very well typed. Could be a remnant of the past though! :D

components/channel_move_to_submenu/index.tsx

michelengelen · 2022-11-23T14:26:34Z

components/compass_design_provider/index.tsx

@@ -0,0 +1,26 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.


could we add TODO comments (maybe TODO@LosTigres: ...) so that we can filter out which components we can replace once we reach a point with the component effort where this is possible? That would really help a lot!

i didnt understand sorry.

OK, maybe you are not aware of it, but we are currently restarting the effort of building a component library ... this time it will be based on Material UI and since we will handle a lot of components with this it would be great to know which components got recently added that mimik that compass design look and feel so we can start replacing those once we reach a point where we have a sufficient amount of components ready

@michelengelen i meant where do you want the todo comment to be and what it should include?

components/menu/index.ts

components/menu/menu_item.tsx

components/menu/sub_menu.tsx

components/sidebar/sidebar_category/sidebar_category_menu/sidebar_category_menu.tsx

fix in middle

components/channel_move_to_submenu/index.tsx

hmhealey · 2022-11-24T22:00:57Z

components/menu/menu.tsx

+    };
+
+    return (
+        <CompassDesignProvider theme={props.theme}>


Do we need to target individual menus with this? I would've thought that we'd wrap the whole app in this similar to how we do the react-intl context

i have tried that but somehow the context of provider placed in root is not reaching way down in the component. 🤔

I wonder if it's similar to the old OverlayTrigger that doesn't pass context down properly any more. Is it just submenus that it doesn't properly apply to?

Its strange that it does not pass it down. I wonder what could be hindering it from doing so. Would you mind trying to add this to the routing level? The last time I checked that was the place where I tried to add it and it worked.

components/menu/menu.tsx

hmhealey · 2022-11-24T22:12:59Z

components/sidebar/sidebar_category/sidebar_category.tsx

@@ -274,8 +274,6 @@ export default class SidebarCategory extends React.PureComponent<Props, State> {
            categoryMenu = (
                <SidebarCategoryMenu
                    category={category}
-                    isMenuOpen={this.state.isMenuOpen}
-                    onToggleMenu={this.handleMenuToggle}


I noticed the styling for this component changes when the menu is open. Is that something we'll need to worry about with this? It's possible that extra CSS class that's applied is for dealing with overflow though which we shouldn't have to do anymore

components/menu/index.ts

package.json

hmhealey · 2022-11-24T22:25:33Z

webpack.config.js

@@ -247,6 +247,7 @@ var config = {
            'mattermost-redux/test': 'packages/mattermost-redux/test',
            'mattermost-redux': 'packages/mattermost-redux/src',
            reselect: 'packages/reselect/src',
+            '@mui/styled-engine': '@mui/styled-engine-sc',


Why do we need an alias for this? I would've expected this to just work out of the box without any sort of import aliasing

Yeah material ui works out of box for emotion-css. To make it work with styled components we need this alias. https://mui.com/material-ui/guides/styled-engine/

Hopefully this won't break when we start to using the components package elsewhere. We'll probably need to look at how Babel handles this, but that can be done later when we move this into the components package.

CC @michelengelen

…into MM-47663-b

…M-47663-b

M-ZubairAhmed · 2022-12-04T13:56:33Z

will be handled in separate individual prs. thanks everyone.

mm-cloud-bot · 2022-12-04T13:56:39Z

Test server destroyed

mm-cloud-bot · 2022-12-04T13:56:40Z

Test server destroyed

Nevyana Angelova added 30 commits July 26, 2022 13:57

Upgrade React and relevant packages to v17

d8bddcd

Merge branch 'master' into MM-34187

8d4711a

dependency package

c72e8e0

add package lock

100a78a

Merge branch 'master' into MM-34187

3285f1b

Fix lint

888045c

Update react

2e08992

Merge branch 'master' into MM-34187

f66ba52

remoe cypress change

771ab9e

Don't scroll RHS when emoji picker is open

fc8bc5e

Merge branch 'master' into MM-34187

03c6ffe

skip integrity check

8a7ae34

Merge branch 'master' into MM-34187

df00bef

check ts version

df02b9a

Merge branch 'master' into MM-34187

76f5433

Updates

e666869

skip integrity AGAIN

150fbfc

skip ts and lint checks for the components package node modules

11bf3f4

dd package-lock

8f72ffd

skip check

3fd2b7d

change typescript plugin for rollup build

d4e3652

remove changes from ts config

8edaaad

PR comments

171848f

Merge branch 'master' into MM-34187

2e5dcb1

Change up react-router name

e32d0fc

clear cache when building webapp

e6c3962

fix makefile

9a94ea5

update circle ci

dbc5810

skip check

ae65e92

rm node modules before lint command

f16d28a

M-ZubairAhmed requested a review from hmhealey November 11, 2022 15:54

M-ZubairAhmed added 5 commits November 22, 2022 19:20

Merge branch 'master' of github.com:mattermost/mattermost-webapp into M…

8551831

…M-47663-b

category_menu_items & sidebar_channel_menu

2c9607b

rename category_menu_items to ChannelMoveToSubmenu

d5d6604

sidebar_category_menu

9849451

sidebar_category_sorting_menu

37c2314

michelengelen reviewed Nov 23, 2022

View reviewed changes

M-ZubairAhmed and others added 9 commits November 24, 2022 20:43

add loading indicator in RHS

4ea22e8

center the loading indicator in Center and RHS

d423eb8

fix in middle

fix bug of saved post crash

1f92f4f

selective import utils

746e203

remove unneccasry listener

a5d978d

tests and snap updates

49a4fe5

e2e

d0661cf

Merge branch 'master' into MM-48573-a

a785a02

Merge branch 'master' into MM-48573-a

c6d2678

hmhealey reviewed Nov 24, 2022

View reviewed changes

M-ZubairAhmed added 6 commits November 25, 2022 15:47

Merge branch 'MM-48573-a' of github.com:mattermost/mattermost-webapp …

b864590

…into MM-47663-b

Merge branch 'master' of github.com:mattermost/mattermost-webapp into M…

20a09ba

…M-47663-b

rev comments

056a932

Update package-lock.json

12a9547

menu modla

fea1a54

sub menu in mobile

d1aa6c5

M-ZubairAhmed closed this Dec 4, 2022

mattermod removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Dec 4, 2022

M-ZubairAhmed deleted the MM-47663-b branch December 4, 2022 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-47663 : Test out MUI for menu components POC #11521

MM-47663 : Test out MUI for menu components POC #11521

M-ZubairAhmed commented Nov 3, 2022 •

edited

hmhealey commented Nov 11, 2022

michelengelen left a comment

michelengelen Nov 23, 2022

hmhealey Nov 24, 2022

M-ZubairAhmed Nov 25, 2022

michelengelen Nov 29, 2022

michelengelen Nov 23, 2022

M-ZubairAhmed Nov 25, 2022

michelengelen Nov 29, 2022

M-ZubairAhmed Nov 29, 2022

hmhealey Nov 24, 2022

M-ZubairAhmed Nov 25, 2022

hmhealey Nov 28, 2022

michelengelen Nov 29, 2022

hmhealey Nov 24, 2022

hmhealey Nov 24, 2022

M-ZubairAhmed Nov 25, 2022

hmhealey Nov 28, 2022

M-ZubairAhmed commented Dec 4, 2022

mm-cloud-bot commented Dec 4, 2022

mm-cloud-bot commented Dec 4, 2022

		@@ -0,0 +1,26 @@
		// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.

MM-47663 : Test out MUI for menu components POC #11521

MM-47663 : Test out MUI for menu components POC #11521

Conversation

M-ZubairAhmed commented Nov 3, 2022 • edited

Summary

Ticket Link

Related Pull Requests

Screenshots

Release Note

hmhealey commented Nov 11, 2022

michelengelen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

M-ZubairAhmed commented Dec 4, 2022

mm-cloud-bot commented Dec 4, 2022

mm-cloud-bot commented Dec 4, 2022

M-ZubairAhmed commented Nov 3, 2022 •

edited