Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Popover] function for slotProps.paper isn't working #42310

Closed
jedwards1211 opened this issue May 20, 2024 · 3 comments · Fixed by #42369
Closed

[Popover] function for slotProps.paper isn't working #42310

jedwards1211 opened this issue May 20, 2024 · 3 comments · Fixed by #42369
Assignees
Labels
bug 🐛 Something doesn't work component: Popover The React component. ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented May 20, 2024

Steps to reproduce

Link to live example: (required)

Steps:

  1. Click the "Open Popovers" button
  2. Popovers should appear above and below the button
  3. Note that the popover above (which uses object slotProps.paper) has a red background
  4. Note that the popover above (which uses function slotProps.paper) has a white background

Current behavior

The popover passed a function for slotProps.paper doesn't have the same appearance as the one with an object for slotProps.paper

Expected behavior

Both popovers have the same background color from slotProps.paper

Context

I was trying to accommodate these changes in MUI in material-ui-popup-state, and a user reported this issue: jcoreio/material-ui-popup-state#134

Your environment

npx @mui/envinfo
  Browser: Chrome 124.0.6367.207
  System:
    OS: macOS 14.4.1
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: 8.11.0 - ~/.nvm/versions/node/v20.10.0/bin/pnpm
  Browsers:
    Chrome: 124.0.6367.209
    Edge: Not Found
    Safari: 17.4.1
  npmPackages:
    @emotion/react: ^11.10.5 => 11.10.5 
    @emotion/styled: ^11.10.5 => 11.10.5 
    @mui/icons-material: ^5.10.16 => 5.10.16 
    @mui/material: ^5.15.18 => 5.15.18 
    @mui/styles: 5.15.14 => 5.15.14 
    @mui/types: ^7.2.2 => 7.2.2 
    @types/react: ^18.0.26 => 18.0.26 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^5.1.0 => 5.4.2 

Search keywords: popover,slot-props

@jedwards1211 jedwards1211 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 20, 2024
@jedwards1211 jedwards1211 changed the title [Popover] function slot props don't work [Popover] function for slotProps.paper doesn't work May 20, 2024
@jedwards1211 jedwards1211 changed the title [Popover] function for slotProps.paper doesn't work [Popover] function for slotProps.paper isn't working May 20, 2024
@jedwards1211 jedwards1211 changed the title [Popover] function for slotProps.paper isn't working [Popover] function for slotProps.paper isn't working May 20, 2024
@zannager zannager added the component: Popover The React component. label May 21, 2024
@mnajdova
Copy link
Member

Thanks for the report, looks like we missed handling function for the slotProps.paper, e.g. I see https://github.com/mui/material-ui/blob/next/packages/mui-material/src/Popover/Popover.js#L128. @DiegoAndai can you please check this?

@mnajdova mnajdova assigned DiegoAndai and unassigned mnajdova May 23, 2024
@DiegoAndai DiegoAndai added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 23, 2024
@DiegoAndai
Copy link
Member

Thanks for the report, @jedwards1211; I accepted it as a bug.
As the Popover is not using the useSlot hook, it's missing the callback functionality.
Ready to take it if anyone wants to work on it. Added it to #40417.

@DiegoAndai DiegoAndai added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label May 23, 2024
@sai6855
Copy link
Contributor

sai6855 commented May 24, 2024

Here's the PR for the fix #42369

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Popover The React component. ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants