pmndrs / leva

🌋 React-first components GUI
https://leva.pmnd.rs
MIT License
4.99k stars 198 forks source link

Custom class/id for controls #97

Closed js2702 closed 3 years ago

js2702 commented 3 years ago

I'm trying to set a class for some controls in order to write some integration tests but can't find any way to do that. I tried looking in the examples + code but didn't find anything. Is it possible? If not it would be nice having a way to add it when setting up the hook.

Thanks for this awesome library :)

gsimone commented 3 years ago

You mean class-per-control, per-panel? Also, would you be interested in contributing tests to the repo? 😄

js2702 commented 3 years ago

Both would be useful for my usecase. Class per-panel would be useful to hide/show programmatically in snapshot tests while class-per-control would be useful to interact with the controls via puppeteer.

How do you think the API would be like? Because not all controls are an object (like the number one), so maybe providing an "style" parameter with the control id as a key would be a way to do it?

About the tests, I'd like to but I don't think my Typescript-fu is strong enough. After looking at the source code I wouldn't know where to start 😢 .

dbismut commented 3 years ago

@js2702 I wrote most of the thing and I don't know where to start either 😅

Anyway, jokes aside, I'm not a test fanatic, but I think users consuming the library shouldn't test the library, and should just expect the library to be tested and work properly (which we is something we need to do).

In other words, Leva (us) should be responsible for making sure the AI matches the data. The user should only be responsible for testing the data. Does that make sense?

Or maybe Jorge you can give us an example where testing the actual UI makes sense?

davidmartos96 commented 3 years ago

@dbismut Colleague of Jorge here. The issue is not about testing Leva itself, but about interacting with the Leva controls from integration tests driven by puppeteer. We use those as it's the only reliable way we found to test a react three fiber canvas. We make snapshots tests that check if the mouse gestures and interactions with the canvas work ok.

While driving those tests we sometimes need to programmatically change a control value (a select or a range value for instance). We can use CSS selectors to find the HTML elements but at the moment there isn't an easy way to just provide an ID or a class to a specific control.

gsimone commented 3 years ago

I think your case could easily be solved by directly mutating the leva store, @dbismut can link you a good sandbox with an example, although the API is still ever-changing, so make sure not to over-commit until we are more stable (unless you are ok with the roughness of pre-launch, which we super appreciate)

js2702 commented 3 years ago

@gsimone I'm not sure if you can mutate the store using puppeteer as you can only interact with the application via clicks/etc as far as I know.

Also we are OK with breaking changes, we are on an early phase of development.

gsimone commented 3 years ago

You could expose the store with global or window maybe. BTW, if you guys want, we can also chat on the pmndrs discord https://discord.gg/ZZjjNvJ we have a dedicated channel

dbismut commented 3 years ago

Here's the sandbox @gsimone is referring to: https://codesandbox.io/s/45bkg Using the function based api returns a set function that you can use to update the store imperatively (thus changing the control values).

const [values, set] = useControls(() => inputs)
davidmartos96 commented 3 years ago

@gsimone @dbismut We are currently doing this, which seems to work for now. I don't think we can interact with the store from the integration test driver because it runs in node, not in the browser.

image

dbismut commented 3 years ago

Accessing the store shouldn't be specific to the browser and should work in node.

import { globalStore } from 'leva'

const values = useControls({ mySelectInput: { value : 'todo', options: ['in progress', 'done'] } })

// in tests
globalStore.get('mySelectInput') === 'todo'

I don't know if that helps. Once again, I don't think it should be for users to test that the panel matches the store structure. This is on our end to do, users should focus on their app logic, if that makes sense?

Now if that doesn't suffice we're of course open to make your life easier: could you suggest an API for Leva matching what you have in mind?

dbismut commented 3 years ago

@davidmartos96 @js2702 don't know if this helps, but we've released 0.4 which has id that match the store path of the input. https://codesandbox.io/s/mystifying-bas-xhfkp

image
davidmartos96 commented 3 years ago

@dbismut That should make things easier, thank you! Just a minor thing, what do you think about prefixing the id with "leva_" or something on those lines. Just to avoid possible name clashing with the control names.

dbismut commented 3 years ago

@davidmartos96 sorry for the late answer but 0.4.5 adds a leva__ prefix to input ids. Hope this helps.

davidmartos96 commented 3 years ago

@dbismut Awesome! Thank you!