ant-design: A List of `antd`'s components that cannot work with HOC
It is not a common use case, so it’s low priority. But we still can discuss and try to make it better:
Menu
Menu.SubMenu
Menu.MenuItem
, cannot be wrapped for keyboard operations rely oninstance.onKeyDown
https://github.com/ant-design/ant-design/issues/4849Table.Column
, forTable
will readchildren[n].props
as config and it cannot not read children’s ownee’s props now https://github.com/ant-design/ant-design/issues/4029 https://github.com/ant-design/ant-design/issues/4324Collpase.Panel
, forCollapse
will readchildren[n].header
as title.Tabs.TabPane
, forTabs
will readchildren[n].tab
to generate TabBar. https://github.com/ant-design/ant-design/issues/5165Select.Option
, forSelect
depends onkey
prop on Option.
Descriptions.Item
https://github.com/ant-design/ant-design/issues/27411
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 61
- Comments: 75 (20 by maintainers)
Commits related to this issue
- display fullName if exists, fix menu keys for antd — committed to sysgears/apollo-universal-starter-kit by mitjade 7 years ago
- fix for List and HOC: https://github.com/ant-design/ant-design/issues/4853 — committed to PaulLaux/eth-hot-wallet by PaulLaux 6 years ago
- fix for List and HOC: https://github.com/ant-design/ant-design/issues/4853 — committed to soft-proactive/eth-hot-wallet by PaulLaux 6 years ago
- fix for List and HOC: https://github.com/ant-design/ant-design/issues/4853 — committed to davidsoft318/eth-hot-wallet by PaulLaux 6 years ago
- fix for List and HOC: https://github.com/ant-design/ant-design/issues/4853 — committed to soft-passion/eth-hot-wallet by PaulLaux 6 years ago
Not fan of codemod solution. Just my point of view, but, I believe that sadly for most of people it is more repulsive than welcomed.
Embedding components into custom components is a common practice in React.
Please fix it and see ant-design adoption fast increase with no doubt.
Ant Design is really appealing and is the first library that makes me want to leave react-bootstrap.
This issue should really be mentioned in the docs, I wasted a full workday researching this bug only to understand this library is unusable for me. Please please please put a warning so other developers won’t have to waste so much time figuring out you used an anti-pattern, relaying on React keys, and now you are very fragile to so many HOC or decorators available to the community. This is really a shame. I hope you’ll fix it in your next major version.
Totally something that should be really fast considered as high priority. 🔥
I spent some time on that issue…
and It’s doable 🎉 , you just need to pass rest props to ie. Collapse.Panel (check source code of rc-panel to understand it)
and then use it like that:
@benjycui I was investigating the workaround you proposed but I think it is not a proper solution. Usually when you wrap a component you want also to have some internal state. With the solution proposed this is not possible. Also I think this is not a little problem. Not being able to isolate common components means having the same code repeated lot of times inside an application. If the application is big I would consider not to adopt antd at all. Please consider this as a constructive critic. Thanks for your work!
Agreed to say this is not a small problem and it’s something I did not expect of when I started using Ant Design library, the good practice of custom components are used throughout React projects. Personally, I really like Ant Design, but for some, this could be a dealbreaker. Would really love to see this gets improved in the upcoming Ant Design v3.
I can’t believe there is still no fix for that.
For Typescript users out there:
If you want to wrap inner components like
Collapse.Panel
with custom behaviour, all you have to do is addCollapsePanelProps
to the props definition of your wrapper component and then spread the props in the actual Panel component that is returned. i.e -You can see what’s happening under the hood here: The
Collapse
clones all of its direct children (which it expects to be Panel) and injects additional props into them. So all you need to do is allow any arbitrary direct child ofCollapse
to accept those props.You can also override whichever props you want like
isActive
, just make sure you add it after the{...props}
spread. This will allow you to have granular control over thePanel
’s behaviour. I.e -This really needs to be fixed. Most developers dont just use components straight out of the box. This means that we might want to integrate more functionality that cannot be otherwise added trhough the api provided. I would not call HOC’s a non common or low priority use case. It is fundamental to the compositional nature of React.
Please, fix this, and while its being fixed add the information, as well as the workarounds people have found here into your official docs.
This is absolutely an high priority. Literally makes those components unusable when special behaviors are required (sometimes even simple ones, like authorization). For example, I’m trying to create a dynamically-loaded menu, so I literally display a disabled
Menu.Item
with a spinner as its name until the data arrives. That’s far from optimal.I would have definitely appreciated mention of this in the documentation! I was trying to do something like this and wasted quite a bit of time because it doesn’t work.
Where
MyCollapseItem
&MyCollapseItem2
render theCollapse.Panel
.Also, now that react16 allows you to return arrays of components from render, being able to do this would be especially helpful. Otherwise, prevent duplicate code is challenging.
Any update on this?
Definitely needs acknowledgement in the documentation. Seriously frustrating to come across this issue (among quite a few other minor ones) that are making me seriously reconsider using AntDesign in my projects.
Yes, we need to
context
instead ofcloneElement
to solve the problem.Thought I’ld document this for those who are trying to make Collapse work with a custom component: Similarly to what @jfreixa mentioned, just pass all the props given to the custom component to the Panel, eg
IMHO, this thread should be in the official docs, because this issue is likely to be a dealbreaker for many users. The docs should also mention https://github.com/react-component/collapse/issues/73#issuecomment-323626120 as an alternative when no state is needed.
Please find a solution for this in v3.
this issue may actually drive me away from antd… Or at least cause me to find a replacement for these components. It really limits your ability to create reusable components.
Still no workaround for Descriptions.Item ?
I believe I found the solution which also fixes menu items not highlighting, a combination of replies in this thread (thanks!) – passing {…props} AND using the same key for the wrapper component as Menu.Item. A trimmed down example:
In addition to previous comments - yes I also think that this could be considered as high priority. After I successfully solved one of my issues (as I described above), now I would need to create
Tabs.TabPane
component wrapped with my custom component. I have very common use-case -> wrapping component is used to check permissions, so if conditions are met, child component is rendered, otherwise not.Is there any simple and working workaround for this?
I found a workaround basically you can pass the rest of the props with <Tabs.TabPane {…props} /> from the wrapped component.
For posterity, I thought I’d share a workaround (hack?) that partially works for Tabs and may work for other components that ant-design doesn’t allow to be extended by an HoC. Take a look at this example.
As you can see, it requires the wrapper component to be called in a function manner rather than via JSX. It’s an atypical DX, for sure… The biggest remaining limitation for me, though, is that you can’t control the
className
at the tab level; you can only wrap the tab contents with a class. So this workaround may or may not work for you, but I thought I’d share.This issue need to fix. MANY (or… almost) developers wants to use library to custom Components.
Example) This code is not working. Because Menu and Menu Item need Opaque props.
is there any update about this issue ? Menu item is behaving weird with HOC.
I don’t think this is a good idea.
Hi ! Same here, I’m really interested in being able to customize these kind of components. Actually having the problem with a custom
Select.Option
No workaround proposed in this thread helped me make it work, I have a working select with empty options…
Excuse me if this is a stupid question, but I am still fairly new to React and Ant Design. Does this mean that practically we can’t use Ant design menus inside a react-redux connected SPA? If so, how can you write a relatively complex SPA with Ant Design? Is there a workaround?
This is actually a major issue for us. I’d vote for the renaming breaking change, as there is no workaround I can think of.
https://codesandbox.io/s/panel-separated-collapse-forked-67495r?file=/PanelElement.js
In my case, I have to put “header ={calculated_header}” behind “{…props}”. If I don’t do this, the header of panel doesn’t show. I think “{…props}” which appears later in the sequence overwrites the “header” information. When I put “{…props}” at the beginning, it works. In that case, I think the “header” which appear later overwrites the “header” information in the props.
My adaptation to @marcin-piela response is as follows:
Can you take a look at my example above? I thought I applied every work around in this thread but it still doesn’t work properly. I’m passing the props down and spreading them on the
Menu.Item
but it still won’t highlight when active, and the component tree looks like thisHere’s what worked for me to get custom components + conditional rendering for
react-router
NavLinks
inside anantd
Menu
, really though this needs a fix - I wasted so much time before finding this thread.edit- Nevermind, selectedKeys doesn’t work properly
@moonjoungyoung @adamerose did you read the thread ?
You have to pass rest props to the inner antd component to make it work.
As I understand it, the parent Collapse will need to set props other than header, key, and extra (from your non working example). These props from the parent Collapse need to be explicitly put onto the Panel Component within your custom component.
I guess you could use the React Inspector to learn all of the possible props that will change and pass them in one by one, but the …props syntax makes sure that anything the parent wants to add to its child Panel will be attached (including but not limited to the ones you are explicitly needing to set)
Maintainers are open to PR, you can also send a PR for a docs update if you want and have the time to do it.
Just encountered this issue FYI.
Would be awesome to see this solution in the 4.0 release!
@gibelium I think that icon rendering deserves its own issue actually. I cloned your repo and tried replacing the icon with a ghost button and the button outline is visible, but the icon also does not render in/on the button…
I think we have a way to remove the dependency of the key. Take Menu as an example, we can introduce an
itemKey
prop and then use thecontext
to implement the Menu. In order to maintain compatibility, the Menu still traverses the children and change thekey
toitemKey
if it presents. At the same time, we can also keep the semantics of props such asselectedKeys
.Temporary workaround (edited for simplicity) for Submenus:
When dealing with arrays:
eventKey
andsubMenuKey
, which should be uniqueA better approach would probably be:
Usage:
Again, you probably do not want to use the index in the array, so don’t use this in production, but the idea is solid.
@Nomeyho I ended up reconstructing the logic that determines whether the
ant-menu-submenu-selected
class gets added by copy/pasting the relevant methods and commenting out the original check, then passing the class asclassName
in my wrapper:If you use with
React.Children.map(children, (child, index) => { ... })
the workaround doesn’t work, at least for me. In the end, the key property gets a value likemykey/.0
which is not what I can work. I’d also vote for a name change on the properties.Same here - I need that for
Tabs.TabPane
andMenu.MenuItem
.