search

dataarts / dat.guiVR

A flexible graphical user interface for changing variables within WebVR.
https://workshop.chromeexperiments.com/examples/guiVR/
Apache License 2.0
311 stars 50 forks source link

Change folder layout to make nested folders work. #80

Closed xinaesthete closed 7 years ago

xinaesthete commented 7 years ago

More generally, any GUI element with a 'spacing' property will be laid out to use that much vertical space rather than the default 'spacingPerController', so it's now possible to make taller (or shorter) controls.

Folders now have additional properties 'isFolder', 'hideGrabber' & 'performLayout'. Minor inconsequential change to button - changing name of function from createCheckbox to createButton. I removed a seemingly extraneous level of THREE.Group around each gui element added to folders. Perhaps it may have been more appropriate to not fix what wasn't broken, but seemed like a minor and safe change in the direction of making things somewhat simpler. Some other various comments dotted about (some with my initials PJT).

This single commit replicates changes that were originally mixed up with some other changes not suitable for merging upstream. I'm sure there'd be a cleaner more idiomatically git-ish way to do this (I need to learn git better), but the result should hopefully be a merge-able implementation of the feature in question (or at least close; maybe some comment noise could be stripped etc).

mflux commented 7 years ago

This is great, thank you for this contribution. I'm reviewing it now.

xinaesthete commented 7 years ago

Sounds reasonable. As I commented above, I thought a lot of the comment noise may not really be appropriate - was mostly for myself as I was making sense of things poking around (not really intended as suggestions, although in some cases worded as such).

xinaesthete commented 7 years ago

I think I've addressed both points now (well, I didn't bother to add any rejection of folders passed to addController), LMK any other feedback etc. Actually, come to think of it, while I agree with addFolder being the principle method for adding folders, I'm not sure whether or not they should also count as controllers for the purposes of passing to addController. There might be times down the line when it is useful for client code not to need to discriminate.

Cheers

mflux commented 7 years ago

Merged. Awesome! Hooray nested folders.

Your recursive binding from object map to controllers is also really cool too. Perhaps we can turn that into a gui.parse( obj ) and it would just build controllers out of that.

xinaesthete commented 7 years ago

Indeed, perhaps with options for filtering out some properties. I expect some of my use of this library to be fairly heavily data driven.

Anyway, my pleasure, thanks for being receptive to changes.

xinaesthete commented 7 years ago

Also note, I believe this should also close #67