Prop passing suggestion
reintroducing opened this issue · 8 comments
I read the documentation about not passing through props, but would it be possible to at the least pass through data-* props? These should be safe to pass through as they don't alter/affect the functionality of the component. The current use case for us is that we use data-test attributes to get cleaner tests written for our components (and it's a paradigm heavily favorite by Testing Library) but we cannot drill those attributes down into the react-collapse elements.
My suggestion would be the provide a data object in much the same way as you already do theme which can then take attribute name/value pairs and translate that to data attributes on the component. you can even provide a prop to change the data-* attribute name for more fine grained control. A crude example:
...
theme={{collapse: 'foo', content: 'bar'}}
data={{collapse: 'test-foo', content: 'test-bar'}}
dataAttribute="test"
...This would translate to something like:
<div data-test="test-foo">
...
If dataAttribute would be set to `baz' then the markup would be:
<div data-baz="test-foo">
...
Alternately, if you don't want to allow changing the dataAttribute (I'd encourage it since it is settable in Testing Library and would provide greater flexibility, I have a use case around this as well if you need convincing but I'll spare you the details), the attribute name can be parsed from the actual data object, so collapse: 'test-foo' can be parsed into data-test="foo", but obviously you'd have to establish and document the pattern for how its parsed (you could even say something like collapse: 'test|test-foo' would be parsed into data-test="test-foo", but obviously that requires greater explanation.
Regardless, it would be great to implement this on some way/shape/form. Thoughts?
From https://github.com/nkbt/react-collapse#pass-through-props
Pass-through props
IMPORTANT Collapse does not support any pass-through props, so any non-supported props will be ignored
Because we need to have control over when Collapse component is updated and it is not possible or very hard to achieve when any random props can be passed to the component.
Why not use theme for this? I mean it is possible to set any classnames to any of the elements of the component?
<Collapse theme={{collapse: 'foo test-collapse', content: 'bar test-content'}}>
<div>Content here</div>
</Collapse>Yeah it will be an html class attribute in the end
Classnames can be removed the same way any data attributes are removed. Just one example: https://www.npmjs.com/package/babel-plugin-react-remove-classname
@nkbt understood, this is not the responsibility of this library and as such I'll go ahead and close this. we will work around it in our UI library in other ways. Appreciate the discussion.
Thank you for understanding. I am not enjoying rejecting ideas or prs, but at some point I made a promise to myself to not add functionality that can be achieved outside of the library to avoid infinite feature growth. Had a few examples going south like that.
that's totally understandable, an open source project can absolutely not cater to one (or every single) need for a very specific use case unless the maintainer also sees value in it. there are other ways to solve this problem and thus it is not on you to bloat the library rather on us to figure out what those other ways are since this is very specific and obviously not something that has been requested many times over. i appreciate the work you do and i appreciate the ease of use of this library so if there is no value here then there is no reason to make the API any more cumbersome than it needs to be and give you more nightmares when trying to maintain the implementation under the hood.