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

[Accordion] Additional actions in summary #9427

Closed
futbolistua opened this issue Dec 7, 2017 · 38 comments · Fixed by #17969
Closed

[Accordion] Additional actions in summary #9427

futbolistua opened this issue Dec 7, 2017 · 38 comments · Fixed by #17969
Labels
component: accordion This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. hacktoberfest https://hacktoberfest.digitalocean.com/ new feature New feature or request priority: important This change can make a difference

Comments

@futbolistua
Copy link

futbolistua commented Dec 7, 2017

The ExpansionPanelSummary handles click to expand/collapse details panel. But I want to put some actions buttons to it. But it's not possible to disable click on the whole summary panel and handle only expandIcon click.

@oliviertassinari oliviertassinari added component: accordion This is the name of the generic UI component, not the React module! new feature New feature or request labels Dec 7, 2017
@codeskills-nl

This comment has been minimized.

@futbolistua

This comment has been minimized.

@stevewillard
Copy link
Contributor

@futbolistua were you able to work around this? I'm also trying to prevent the expansion by clicking on the summary (I just want to expand when you click on the icon).

@chathuraa

This comment has been minimized.

@stevewillard
Copy link
Contributor

You can do what @codeskills-nl mentioned

  clickSummary = event => {
    event.stopPropagation();
  };

...

<ExpansionPanel onClick={this.clickSummary}>

@chathuraa
Copy link

Hi @stevewillard,

Thanks for the suggestion. I have an MUI checkbox in the summary, so when I do stopPropagation, it's messing up check/unchecking the checkbox.

Thanks,
Chat

@Loktor
Copy link

Loktor commented Jan 30, 2018

There is a way to get exactly the behavior you guys want (should also solve your problem @chathuraa):

<ExpansionPanel expanded={this.state.expansionPanelOpen}>
             <ExpansionPanelSummary expandIcon={<ExpandMoreIcon onClick={() => {
               this.setState({
                 expansionPanelOpen: !this.state.expansionPanelOpen
               });
             }}/>}>
               ....

You can manage the ExpansionPanel expanded state yourself, and by using the onClick event of the Icon which is passed to the ExpansionPanelSummary (which is the header line) you can open/close the expansion panel only via the icon)

@TheMoonDawg
Copy link
Contributor

Amazing workaround Loktor!

@kamranayub
Copy link
Contributor

kamranayub commented May 14, 2018

This is pretty common in even regular JS. One way to mitigate this that we're doing is ensuring all input components have a common wrapper. Then you can add the onClick handler to the wrapping element and once the click event reaches it, you can stop propagating.

/* see: https://github.com/mui-org/material-ui/issues/9427 */
const stopPropagation = (e) => e.stopPropagation();
const InputWrapper = ({ children }) =>
  <div onClick={stopPropagation}>
    {children}
  </div>

// usage:

<InputWrapper>
  <MyCoolInput />
</InputWrapper>

This would be a decent way to handle it, if you need to have the summary be clickable. This lets you control propagation selectively if you wish too. Otherwise @Loktor's method works well too, you just might need to override the cursor styles and some attributes because the Summary is still tabbable and has aria roles on it.

@leolozes
Copy link

leolozes commented Nov 7, 2018

Thanks @kamranayub your solution is the best here.
It would be nice to have "actions" in the ExpansionPanelSummary (that could be aligned left or right maybe?) where the default behaviour is like this, even if it's easier now with this solution.

@Usama-Tahir
Copy link

There is a way to get exactly the behavior you guys want (should also solve your problem @chathuraa):

<ExpansionPanel expanded={this.state.expansionPanelOpen}>
             <ExpansionPanelSummary expandIcon={<ExpandMoreIcon onClick={() => {
               this.setState({
                 expansionPanelOpen: !this.state.expansionPanelOpen
               });
             }}/>}>
               ....

You can manage the ExpansionPanel expanded state yourself, and by using the onClick event of the Icon which is passed to the ExpansionPanelSummary (which is the header line) you can open/close the expansion panel only via the icon)

This works fine. But I have an array which I populate using map function. Let's say I have 5 expansion panels and after clicking on the icon button, I change the state as mentioned in your answer. However, the problem with the approach is that it expands all expansion panels but I only want to expand the expansion panel being clicked

@b-ferreira
Copy link

b-ferreira commented Jan 31, 2019

There is a way to get exactly the behavior you guys want (should also solve your problem @chathuraa):

<ExpansionPanel expanded={this.state.expansionPanelOpen}>
             <ExpansionPanelSummary expandIcon={<ExpandMoreIcon onClick={() => {
               this.setState({
                 expansionPanelOpen: !this.state.expansionPanelOpen
               });
             }}/>}>
               ....

You can manage the ExpansionPanel expanded state yourself, and by using the onClick event of the Icon which is passed to the ExpansionPanelSummary (which is the header line) you can open/close the expansion panel only via the icon)

This works fine. But I have an array which I populate using map function. Let's say I have 5 expansion panels and after clicking on the icon button, I change the state as mentioned in your answer. However, the problem with the approach is that it expands all expansion panels but I only want to expand the expansion panel being clicked

@Usama-Tahir

You can use an arbitrary condition based on a local state to decide what panel will be opened. For example:

state = {
  panel: ''
};

const handleStateChange = panel => () => this.setState({ panel});

// This would be your list of available panels, 
// which I suppose you go through to create your panels.
const panels = {
  panelOne: 'panelOne',
  panelTwo: 'panelTwo',
};

Object.keys(panels).map(panel => (
  <ExpansionPanel expanded={this.state.panel === panel}>
    <ExpansionPanelSummary
      expandIcon={<ExpandMoreIcon onClick={handleStateChange(panel)} />}
    >
      ...
    </ExpansionPanelSummary>
    ...
  </ExpansionPanel>
));

@Usama-Tahir

This comment has been minimized.

@sibelius
Copy link

I have a checkbox inside ExpansionPanel

I'm doing this on onChange of Checkbox

event.stopPropagation();
event.preventDefault();

but the ExpansionPanel keep expanding

can I stop this behaviour?

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 13, 2019

Also please remove next style .MuiExpansionPanelSummary-content-35 > :last-child {
/* padding-right: 32px; */
}

Done: #14828.

@Usama-Tahir
Copy link

let's say I want to open expansion panel by clicking anywhere on the panel (which is the default behavior). But there is a button inside my expansion panel. When I click on that button, I don't want to open the expansion panel. How Can I achieve this? I tried to use z-index but I didn't help.

@b-ferreira
Copy link

let's say I want to open expansion panel by clicking anywhere on the panel (which is the default behavior). But there is a button inside my expansion panel. When I click on that button, I don't want to open the expansion panel. How Can I achieve this? I tried to use z-index but I didn't help.

@Usama-Tahir it was already covered by this comment from @kamranayub.

@Usama-Tahir
Copy link

I don't know how that solves my problem.

@oliviertassinari
Copy link
Member

I would try to do the distinction between event.currentTarget and event.target.

@b-ferreira
Copy link

b-ferreira commented Mar 19, 2019

@Usama-Tahir if you are talking about having a button into ExpansionPanelSummary which should not open the ExpansionPanel when it's being clicked, then the comment from @kamranayub totally solves your problem.

It's related to the event.stopPropagation() method.

const _handleButtonClick = event => {
  event.stopPropagation();
  ... Do your stuff after here.
}

<ExpansionPanel>
  <ExpansionPanelSummary>
    <Button onClick={_handleButtonClick}>Click me</Button>
  </ExpansionPanelSummary>
  ...
</ExpansionPanel>

This should avoid the ExpansionPanel to being opened when you click onto the Button.

Does that make sense to your issue?

@Usama-Tahir
Copy link

yes, That solved my problem partially. Actually, I render expansion panels from an array. Initially, I was using expanded prop inside <ExpansionPanel /> to open it. So when I clicked on another expansion panel, it would close any other opened expansion panel as well. I can't figure out how to achieve this with the current solution you provided?

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. and removed waiting for user information labels Mar 21, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 21, 2019

What do you guys think of this approach?

--- a/docs/src/pages/demos/expansion-panels/ControlledExpansionPanels.js
+++ b/docs/src/pages/demos/expansion-panels/ControlledExpansionPanels.js
@@ -1,8 +1,8 @@
 import React from 'react';
-import { makeStyles } from '@material-ui/core/styles';
+import { makeStyles, withStyles } from '@material-ui/core/styles';
 import ExpansionPanel from '@material-ui/core/ExpansionPanel';
 import ExpansionPanelDetails from '@material-ui/core/ExpansionPanelDetails';
-import ExpansionPanelSummary from '@material-ui/core/ExpansionPanelSummary';
+import MuiExpansionPanelSummary from '@material-ui/core/ExpansionPanelSummary';
 import Typography from '@material-ui/core/Typography';
 import ExpandMoreIcon from '@material-ui/icons/ExpandMore';

@@ -21,18 +21,28 @@ const useStyles = makeStyles(theme => ({
   },
 }));

+const ExpansionPanelSummary = withStyles({
+  root: {
+    cursor: 'default',
+  },
+})(MuiExpansionPanelSummary);
+
 function ControlledExpansionPanels() {
   const classes = useStyles();
   const [expanded, setExpanded] = React.useState(null);

-  const handleChange = panel => (event, isExpanded) => {
-    setExpanded(isExpanded ? panel : false);
+  const handleChange = panel => () => {
+    const isExpanded = expanded === panel;
+    setExpanded(isExpanded ? false : panel);
   };

   return (
     <div className={classes.root}>
-      <ExpansionPanel expanded={expanded === 'panel1'} onChange={handleChange('panel1')}>
+      <ExpansionPanel expanded={expanded === 'panel1'}>
         <ExpansionPanelSummary
+          IconButtonProps={{
+            onClick: handleChange('panel1'),
+          }}
           expandIcon={<ExpandMoreIcon />}
           aria-controls="panel1bh-content"
           id="panel1bh-header"
@@ -47,8 +57,11 @@ function ControlledExpansionPanels() {
           </Typography>
         </ExpansionPanelDetails>
       </ExpansionPanel>
-      <ExpansionPanel expanded={expanded === 'panel2'} onChange={handleChange('panel2')}>
+      <ExpansionPanel expanded={expanded === 'panel2'}>
         <ExpansionPanelSummary
+          IconButtonProps={{
+            onClick: handleChange('panel2'),
+          }}
           expandIcon={<ExpandMoreIcon />}
           aria-controls="panel2bh-content"
           id="panel2bh-header"
@@ -65,8 +78,11 @@ function ControlledExpansionPanels() {
           </Typography>
         </ExpansionPanelDetails>
       </ExpansionPanel>
-      <ExpansionPanel expanded={expanded === 'panel3'} onChange={handleChange('panel3')}>
+      <ExpansionPanel expanded={expanded === 'panel3'}>
         <ExpansionPanelSummary
+          IconButtonProps={{
+            onClick: handleChange('panel3'),
+          }}
           expandIcon={<ExpandMoreIcon />}
           aria-controls="panel3bh-content"
           id="panel3bh-header"
@@ -83,8 +99,11 @@ function ControlledExpansionPanels() {
           </Typography>
         </ExpansionPanelDetails>
       </ExpansionPanel>
-      <ExpansionPanel expanded={expanded === 'panel4'} onChange={handleChange('panel4')}>
+      <ExpansionPanel expanded={expanded === 'panel4'}>
         <ExpansionPanelSummary
+          IconButtonProps={{
+            onClick: handleChange('panel4'),
+          }}
           expandIcon={<ExpandMoreIcon />}
           aria-controls="panel4bh-content"
           id="panel4bh-header"
diff --git a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
index 388667124..4ea5a86f1 100644
--- a/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
+++ b/packages/material-ui/src/ExpansionPanelSummary/ExpansionPanelSummary.js
@@ -19,9 +19,7 @@ export const styles = theme => {
       minHeight: 8 * 6,
       transition: theme.transitions.create(['min-height', 'background-color'], transition),
       padding: '0 24px 0 24px',
-      '&:hover:not($disabled)': {
-        cursor: 'pointer',
-      },
+      cursor: 'pointer',
       '&$expanded': {
         minHeight: 64,
       },
@@ -30,6 +28,7 @@ export const styles = theme => {
       },
       '&$disabled': {
         opacity: 0.38,
+        cursor: 'default',
       },
     },
     /* Styles applied to the root element, children wrapper element and `IconButton` component if `expanded={true}`. */

https://codesandbox.io/s/jjy809l6ny

I think that it would make a great demo. Does anyone want to handle it?

@oliviertassinari

This comment has been minimized.

@IssueHuntBot

This comment has been minimized.

@danielo515
Copy link

This issue is still a problem.
Expansion panel summary it's a button, hence it is intercepting all kind of click events. This is even worse on mobile platforms.
I think that the container should have a way to provide controls to the expansion panel, I think is something common.

@oliviertassinari

This comment has been minimized.

@danielo515
Copy link

@oliviertassinari your proposals seems to be focused on solving a different problem.
My problem is that the ExpanionPanelSummary is a giant button. This makes impossible that any of it's children detect click events properly because his parent is "absorbing" them.

@oliviertassinari

This comment has been minimized.

@eps1lon eps1lon changed the title Expansion panel behaviour Additional actions in ExpansionPanelSummary Jun 28, 2019
@rgautier2003

This comment has been minimized.

@joshwooding

This comment has been minimized.

@ericrobinsondev
Copy link

ericrobinsondev commented Sep 28, 2019

@Loktor's solution is great, but unfortunately since it was posted there has been a new warning added to material-ui warning that the clicks on the icon won't be seen by Firefox. Here's an alternative solution:

`const CustomExpansionPanel = ({children, className, initiallyExpanded = false}) => {
const [expanded, setExpanded] = useState(initiallyExpanded);

return (
    <ExpansionPanel
        expanded={expanded}
        className={className}
        onClick={(event) => {
            /* Allow only the <ExpandIcon> related to this Expansion panel to expand it. */
            if (event.target.parentElement
                && event.target.parentElement.parentElement
                && event.target.parentElement.parentElement.parentElement
                && event.target.parentElement.parentElement.parentElement.parentElement) {
                if (((event.target.parentElement.parentElement.parentElement.parentElement === event.currentTarget)
                    && (event.target.nodeName === 'svg')) ||
                    // eslint-disable-next-line max-len
                    ((event.target.parentElement.parentElement.parentElement.parentElement.parentElement === event.currentTarget)
                && (event.target.nodeName === 'path'))) {
                    setExpanded(!expanded);
                }
            }
        }}
    >
        { children }
    </ExpansionPanel>
);

};

@oliviertassinari oliviertassinari added the priority: important This change can make a difference label Sep 29, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 29, 2019

@drericrobinson Could you explain the behavior you are looking for with this logic? I would hope we can simplify it. A significant amount of people have interacted with the issue, it seems to be an important concern.

@eps1lon
Copy link
Member

eps1lon commented Sep 29, 2019

#9427 (comment) already had a simple but potentially inaccessible solution

We can add a demo (without changing the ExpansionPanel implementation) but need to document how to improve a11y as much as possible: https://codesandbox.io/s/material-ui-expansionpanel-nested-action-u2bm6 should fix most of the a11y issues introduced by simply stopping click propagation of the nested actions.

@oliviertassinari
Copy link
Member

@eps1lon This sounds like a great approach, it works with a checkbox too: https://codesandbox.io/s/material-ui-expansionpanel-nested-action-9pkk5. I'm all in for adding a demo for the "action inside panel summary" case.

Looking at the comment history, it seems that people also ask for an alternative where the expandIcon is the only element that can open or close the panel. It's related to #9427 (comment) but it would also need to handle keyboard interactions, need to check the a11y aspect too. Maybe we would need a prop this time.

@eps1lon
Copy link
Member

eps1lon commented Sep 30, 2019

So all that's missing is a reasonable use case for that (not just dummy text). If someone can work on a real example I'd be happy to review PR that adds a demo that incorporates some key aspects from https://codesandbox.io/s/material-ui-expansionpanel-nested-action-u2bm6:

  • click propagation is stopped in the nested action
  • focus propagation is stopped in the nested action (in react focus events bubble)
  • label each action with aria-label or aria-labelledby. Otherwise the label for nested actions will be included in the label of the parent button

@joshwooding joshwooding added the hacktoberfest https://hacktoberfest.digitalocean.com/ label Sep 30, 2019
ayliao added a commit to ayliao/material-ui that referenced this issue Oct 21, 2019
@ayliao
Copy link
Contributor

ayliao commented Oct 21, 2019

Giving this one a hacktoberfest shot here: #17969 :P

@sjk191011

This comment has been minimized.

@zain-ak
Copy link

zain-ak commented Mar 5, 2020

I used the method mentioned by @Loktor and it worked perfectly for me as my expansion panels are in separate components.

My only qualm is the fact that I'm getting this warning/error:
Capture

I've tested my app out in Firefox and it's working as expected, maybe misses the initial click at times but works after that. This'll be a mobile web app so it'll only be running on Chrome and Safari anyhow. Is there anyway I can get rid of this error? And is it fine for me to proceed with the method I'm using?

Below is the snippet of code I'm using; an attribute inside ExpansionPanelSummary:
expandIcon={<ExpandMoreIcon onClick={() => setPanel(!isPanelOpen)}

@oliviertassinari oliviertassinari changed the title Additional actions in ExpansionPanelSummary [Accordion] Additional actions in summary Jul 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: accordion This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. hacktoberfest https://hacktoberfest.digitalocean.com/ new feature New feature or request priority: important This change can make a difference
Projects
None yet
Development

Successfully merging a pull request may close this issue.