Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethink value.value for multiple controls #92

Open
alvarosabu opened this issue Dec 15, 2023 · 10 comments
Open

Rethink value.value for multiple controls #92

alvarosabu opened this issue Dec 15, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request

Comments

@alvarosabu
Copy link
Member

Is your feature request related to a problem? Please describe.
When more than one control is passed to the useControls, the object returned contains all the controls refs, so to access the value inside is slider.value.value

See https://tresleches.tresjs.org/guide/controls.html#multiple-controls

Since vue reactivity uses value for refs and having these differences within single and multiple controls is awkward and worsens the DX

Describe the solution you'd like

@andretchen0 I need your help here, what do you think we can do? Maybe instead of an object with all the controls, let's return an array?

@alvarosabu alvarosabu added the enhancement New feature or request label Dec 15, 2023
@alvarosabu alvarosabu self-assigned this Dec 15, 2023
@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

I think I need to see how others handle this use case. I'll look for some other Vue examples and post back here to continue the discussion.

I need your help here, what do you think we can do? Maybe instead of an object with all the controls, let's return an array?

Here's what we have currently: return object with widgets in object.[widget name]

const state = {reset:true, helper:true}
const {reset, helper} = useControls({reset: true, helper: true})
watch(widgets, () => { 
  state.reset = reset.value.value
  state.helper = helper.value.value
})

Here's the proposal, as I see it: return array of [widget, ...]

const state = {reset:true, helper:true}
const widgets = useControls({reset: true, helper: true})
watch(widgets, () => { 
  state.reset = widgets[0].value.value
  state.helper = widgets[1].value.value
})

I think there'd be a tendency to get lost in the array indices. Imagine having 10 widgets and you decide to add/remove one from the middle. Everything after has to be renumbered.

@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

Example: Tweakpane

Tweakpane | Bindings

Tweakpane uses .addBinding(stateObject, key, params).

const PARAMS = {
  speed: 50,
};

const pane = new Pane();
pane.addBinding(PARAMS, 'speed', {
  min: 0,
  max: 100,
});

With this setup, there's no need for a watch and so no value.value. The value is already being "watched" with the configuration above.

@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

Example: v-tweakpane

A variation of Tweakpane, this time with Vue. Like Tweakpane, it uses a specific method for creating a widget and binding at the same time. This avoids the need to watch and avoids .value.value.

const onPaneTwoCreated = (pane: any) => {
  pane.registerPlugin(CamerakitPlugin);
  const PARAMS = {
    flen: 55,
    fnum: 1.8,
    iso: 100,
  };
  pane.addInput(PARAMS, 'flen', {
    view: 'cameraring',
    series: 0,
    unit: { pixels: 50, ticks: 10, value: 0.2 },
    min: 1,
    step: 0.02,
  });

The example uses some callbacks for setup, which Leches doesn't need. Personally, I'd like to avoid having those.

@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

Example: Vuetify

In this Vuetify example, most of the config that Leches does in <setup> is handled in <template>. Bindings are created using v-model in the <template>.

<template>
  <v-slider
    v-model="slider"
    class="align-center"
    :max="max"
    :min="min"
    hide-details
  >
    <template v-slot:append>
      <v-text-field
        v-model="slider"
        hide-details
        single-line
        density="compact"
        type="number"
        style="width: 70px"
      ></v-text-field>
    </template>
  </v-slider>
</template>

<script>
  export default {
    data () {
      return {
        min: -50,
        max: 90,
        slider: 40,
      }
    },
  }
</script>

@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

Example: Quasar

Here's a slider from UI library Quasar.

The configuration is done in the <template>. No need for watch.

<template>
  <div class="q-pa-md">
    <q-badge color="secondary">
      Model: {{ value }} (-20 to 20)
    </q-badge>

    <q-slider
      v-model="value"
      :min="-20"
      :max="20"
      :step="4"
      snap
      label
      color="purple"
    />
  </div>
</template>

<script>
import { ref } from 'vue'

export default {
  setup () {
    return {
      value: ref(0)
    }
  }
}
</script>

@andretchen0
Copy link

andretchen0 commented Dec 15, 2023

Example: Element UI

Here's a slider example from Element UI.

Another <template> approach with v-model. No need for watch.

<template>
  <div class="block">
    <el-slider
      v-model="value"
      show-input>
    </el-slider>
  </div>
</template>

<script>
  export default {
    data() {
      return {
        value: 0
      }
    }
  }
</script>

Copy link
Member Author

My original idea was to follow the same UX as https://github.com/pmndrs/leva @andretchen0

I agree with you about using an object rather than an array, what if instead of having an object with ref values, we return a reactive object to avoid the .value? I'm not sure that reactivity would work.

@andretchen0
Copy link

what if instead of having an object with ref values, we return a reactive object to avoid the .value? I'm not sure that reactivity would work.

Currently useControls is returning toRefs(reactive(result)) for most of our use cases in the docs and playground, afaik.

It's my understanding that returning reactive(result) would work for this particular use case and eliminate .value.value.

const data = reactive({
  enabled: {value: true},
  frequency: {value: 1, min:0, max:10}
})

watch(data, () => console.log("fired on any change to data (deep)"));

Just to confirm, here's a Vue playground


Just FYI, if the toRefs here is removed, we'd need to redo our demos and playground examples that use Leches. As would other users.

Copy link
Member Author

It will be a breaking change indeed @andretchen0 but since the package is not even in alpha it was expected. That is why I want to think it through to avoid having more breaking changes in the future.

In the case the API does change, we will need to create a ticket to update our demos and playgrounds

@andretchen0
Copy link

andretchen0 commented Dec 19, 2023

breaking change

Gotcha. No problem then.

I think this Leva issue points to a common use case unmet by Leva's approach and also extends to what we're talking about here: the API doesn't help you bind external state. It defaults to creating new state and then the user handles copying the new state values to the external state values.

In the case of Cientos demos, we've been doing e.g., watch with value.value.

From my perspective, it'd be great if we could make binding external state "just work" in Leches. In Vue, a v-model in the <template> could work and also give us two-way data bindings.

If we need to do it in <script> maybe instead of useControls returning keyed widgets – leading to this issue for Leva – it returns a builder API that can be used to further configure the Leches instance:

const myData = {enabled: false}
useControls({enabled: {label:"Enabled"}}).bind(myData)

But maybe that leads us too far away from the original intention here. In which case, no problem.

@alvarosabu alvarosabu changed the title Rethink value.value on multiple controls Rethink API Apr 27, 2024
@alvarosabu alvarosabu changed the title Rethink API Rethink value.value for multiple controls Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants