[List] extract button from ListItem to ListItemButton#26480
Closed
siriwatknp wants to merge 17 commits intomui:nextfrom
Closed
[List] extract button from ListItem to ListItemButton#26480siriwatknp wants to merge 17 commits intomui:nextfrom
siriwatknp wants to merge 17 commits intomui:nextfrom
Conversation
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Member
Author
|
@oliviertassinari @eps1lon please review the API, is it looks good to you? I have one hesitation about naming // option 1
<List>
<ListItemButton ButtonBaseProps={{ component: 'a', href: '#id' }}>
</ListItemButton>
</List>
// option 2
<List>
<ListItemButton ButtonProps={{ component: 'a', href: '#id' }}>
</ListItemButton>
</List>I feel that option 2 might mislead user that the |
Member
@siriwatknp With #21453, the consistent API would be: <List>
<ListItemButton componentsProps={{ button: { component: 'a', href: '#id' } }}>
</ListItemButton>
</List>IMHO, it's not great. The direction taken in #26446 looks better from a DX perspective. |
Member
Author
|
decide to go with #26446 approach. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #13597
closes #26442
Another approach from #26446, please review
ListItemButton.jsSummary
ListItemButtonalways render 2 DOM<li class="root"><div class="button"></li>accept
secondaryActionas prop (wrapped withListItemSecondaryAction) instead ofchildrenbecause it must be the same level as button (not inside).If this api sounds good to the team, will add to
ListItemso that it does not need to iterate children to find secondary actionI have followed (at least) the PR section of the contributing guide.