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

Is there a reason ListItemSecondaryAction is absolutley positioned? #13495

Closed
2 tasks done
Pajn opened this issue Nov 2, 2018 · 8 comments
Closed
2 tasks done

Is there a reason ListItemSecondaryAction is absolutley positioned? #13495

Pajn opened this issue Nov 2, 2018 · 8 comments
Labels
component: list This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@Pajn
Copy link
Contributor

Pajn commented Nov 2, 2018

  • This is not a v0.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

I just run into #8205 for the nth time and my fix is always the same, make ListItemSecondaryAction staticly positioned, make the list item container a flexbox and undo the additional padding.

        <ListItem
          style={{paddingRight: 16}}
          ContainerProps={{
            style: {display: 'flex', alignItems: 'center'},
          }}
        >
          ...
          <ListItemSecondaryAction
            style={{position: 'static', transform: 'none'}}
          >
            ...
          </ListItemSecondaryAction>
        </ListItem>

It would be nice if this was not necessary and instead the default behavior.

Current Behavior

The ListItemSecondaryAction is absolutely positioned, causing the problem that it does not grow to take the space needed for its content.

Examples

N/A

Context

I think this is described with enough context above.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 2, 2018

Is there a reason ListItemSecondaryAction is absolutely positioned?

@Pajn Yes, it's because HTML5 specification prohibits the nesting of <button> elements.

@Pajn
Copy link
Contributor Author

Pajn commented Nov 3, 2018

I'm probably missing some use case, but this doesn't cause nesting of <button> elements.

ListItemSecondaryAction lies next to the ListItem-root, both having <li> as parent.
Event when adding the button property to the list item does not seem to cause a difference, however as ListItem-root no longer extends the full width the hover effect (and click area) is also no longer the full width as it does not stretch below the ListItemSecondaryAction

Not nested:
bild

Button example, not using the full width:
bild

I can't say if not extending the full width when using button is an improvement or a problem. It looks a bit weird but also, having two buttons on top of each other is also weird. The use case ListItem with button + secondary actions feels strange all together. But still, it's a breaking change I guess.

@oliviertassinari
Copy link
Member

I'm probably missing some use case, but this doesn't cause nesting of elements.

@Pajn You are right, I have jumped to the conclusion too quickly. I think that it's important to have a ripple covering the whole surface area. In order to have that, we need to nest the secondary action in the list or to absolutely position the action. If we go with the nesting path, it's important to be careful with the resulting DOM structure, some combinations are invalid. Benchmarking MCW and vuetify it seems we are the only one doing that. I have nothing against exploring the flex layout position path. It's definitely a breaking change, we would have to wait for v4.0.0, coming in the next few months. Feel free to experiment, I think that it will be a success if:

@oliviertassinari oliviertassinari added new feature New feature or request component: list This is the name of the generic UI component, not the React module! and removed discussion labels Nov 4, 2018
@rosskevin
Copy link
Member

rosskevin commented Dec 6, 2018

I tried this long ago and we merged #5911 but reverted due to the ripple issue.

I did try to revisit this, I still have the branch https://github.com/alienfast/material-ui/tree/list-item-flex-revisited - I tried with a ripple bounding element but I did not have success.

I just bumped into this again - am using a ListItem without button but with secondary actions, and want the content to grow but not overflow the actions. I'll do something custom for now but I definitely think revisiting this is worthwhile.

@oliviertassinari
Copy link
Member

I'm closing for #13597 that explores how to improve the ListItemSecondaryAction position.

@Jack-Works
Copy link
Contributor

image

This kind of hard-coded padding-right makes UI overflow when the action is wider than expected

@Jack-Works
Copy link
Contributor

Use a scary method to patch it

    const buttonRef = useRef<HTMLElement>(null)
    const listRef = useRef<HTMLDivElement>(null)
    useLayoutEffect(() => {
        if (!buttonRef.current || !listRef.current) return
        const width = buttonRef.current.getBoundingClientRect().width
        const child = listRef.current.children[0]
        if (!child) return
        ;(child as HTMLElement).style.paddingRight = `${width + 10}px`
    })

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 7, 2021

@Jack-Works If the container isn't a button, I would personally trash the usage of the ListItem components and use custom CSS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants