Buttons with tooltip raise deprecation warning in StrictMode

[eclarke1]

Bug Description

Initially raised as part of the bug bash for dashboard sharing, this issue is not DS-specific and applies to all Button component use with tooltips.

To reproduce, open the browser console and then hover a button that has a tooltip (e.g. user menu, help menu, idea hub action button, etc). They all raise the same warning (with a different trace):

Warning: findDOMNode is deprecated in StrictMode. findDOMNode was passed an instance of Transition which is inside StrictMode. Instead, add a ref directly to the element you want to reference. Learn more about using refs safely here: https://fb.me/react-strict-mode-find-node

Bug bash issue

Bug bash issue: https://app.asana.com/0/1202258919887896/1202436389160100 please refer to Asana issue for background

Not sure exactly how I triggered this but noticed it in the console. We don't use this prop directly anywhere but the trace indicates UserRoleSelect.

Warning: Failed prop type: Material-UI: The `anchorEl` prop provided to the component is invalid.
The anchor element should be part of the document layout.
Make sure the element is present in the document or that it's not display none.
    in ForwardRef(Popper) (created by ForwardRef(Tooltip))
    in ForwardRef(Tooltip) (created by WithStyles(ForwardRef(Tooltip)))
    in WithStyles(ForwardRef(Tooltip)) (created by Button)
    in Button (created by UserRoleSelect)
    in div (created by UserRoleSelect)
    in UserRoleSelect (created by Module)
    in div (created by Module)
    in div (created by Module)
    in Module (created by DashboardSharingSettings)
    in div (created by DashboardSharingSettings)
    in div (created by DashboardSharingSettings)
    in DashboardSharingSettings (created by DashboardSharingSettingsButton)
    in div (created by DashboardSharingSettingsButton)
    in div
    in Unknown (created by DashboardSharingSettingsButton)
    in div (created by e)
    in div (created by e)
    in div (created by e)
    in e (created by DashboardSharingSettingsButton)
    in Portal (created by DashboardSharingSettingsButton)
    in DashboardSharingSettingsButton (created by DashboardMainApp)
    in div (created by Cell)
    in Cell (created by Header)
    in div (created by Row)
    in Row (created by Header)
    in div (created by ForwardRef)
    in ForwardRef (created by Header)
    in header (created by Header)
    in Header (created by DashboardMainApp)
    in DashboardMainApp (created by DashboardEntryPoint)
    in DashboardEntryPoint
    in RestoreSnapshots (created by Root)
    in ErrorHandler (created by Root)
    in Root

Steps to reproduce

Note: this warning will only appear in development builds because it is triggered by React's Strict Mode

1. Go to the Site Kit dashboard
2. Hover a button that has a tooltip (e.g. user menu avatar button)
3. See warning in console

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• Buttons with tooltips should not raise deprecation warnings when interacting with them

Implementation Brief

• Within assets/js/components/Root/index.js:
  • As per this comment, within say <ViewContextProvider>, add <ThemeProvider theme={createTheme}> where createTheme is unstable_createMuiStrictModeTheme() only when in dev mode, otherwise it is createMuiTheme(). <ThemeProvider>, unstable_createMuiStrictModeTheme and createMuiTheme should all be imported from '@material-ui/core'

Test Coverage

• No new tests required.

QA Brief

Changelog entry

[eugene-manuilov]

This is weird issue... I checked our codebase and the material library and everything looked okay to me. Not sure why it happened. @aaemnnosttv what do you think if we ask Darren to try to reproduce this issue and if he can't, then close it?

[eclarke1]

@wpdarren see comment above, could you please try to reproduce this one again?

[aaemnnosttv]

@eugene-manuilov I still see this all the time, do you not see it? 🤔

[jimmymadon]

The summary of all options is given in this comment.

Bumping the estimate to 7 just for testing this change properly as it is a <Root> component change which I am not usually comfortable with. But the only other option is to upgrade to MUI 5.

[aaemnnosttv]

@jimmymadon is there a reason we're not already using v5? It's been released for a while now (Sept 2021), with many releases since then. I'm guessing there's probably a reason but let's rule out this option before we go with your proposed solution to use the unstable API workaround.

cc: @asvinb as I think you were the one to pull in this library as part of the work on GM2+?

[jimmymadon]

@aaemnnosttv Just did a bit of digging and MUI 5 requires React > 17 and we are on 16. Perhaps that is why we are using v4? @asvinb

[aaemnnosttv]

@jimmymadon ah that makes sense. Thanks for checking 👍

[jimmymadon]

@aaemnnosttv Should we move this to execution then? I am not sure if we are planning on bumping up to React 17/18 and if there are any reasons why we haven't done so already. We can use this 'slightly hacky' fix until we do so.

[aaemnnosttv]

@jimmymadon overall it sounds ok to me but would be nice to know a bit more as to what the compromise is here (as far as I can tell it just passes an additional param in the theme config but not clear what it does). We don't use the theme provider currently so I'm guessing it's falling back to a default?

Also, looking at the source it seems that this would raise a new warning in development due to the function being renamed :)

[jimmymadon]

@aaemnnosttv So in v4, it isn't mandatory to use ThemeProvider (it is in v5 from a quick read of the docs). Which warning do you mean - the ESLint Rule for camel case?

[aaemnnosttv]

Which warning do you mean - the ESLint Rule for camel case?

Oops, sorry – I thought I had linked it. Clipboard history to the rescue! 😄 https://github.com/mui/material-ui/blob/a563a60219f7f6519fb0f34f6d8e3bf0974e6495/packages/material-ui/src/styles/createTheme.js#L105-L113

[jimmymadon]

@aaemnnosttv Have updated the IB to use createTheme but as discussed, in production, it wouldn't throw the warning. But better to use createTheme. Thanks.

[aaemnnosttv]

SGTM, thanks @jimmymadon

IB ✅

[wpdarren]

QA Update: ✅

Verified:

• Buttons with tooltips do not raise deprecation warnings when interacting with them.
• I tested this on develop branch and also on the development build.
• I went through all of the pages within Site Kit and did not discover any additional warnings or errors in the console.