Param groups. Collapsable Groups in UI

[protozoo]

• Enable grouping of params with "group" property in param definition object.
• Enable expand/collapse of param groups in UI

This PR implements the feature I requested here: #30
I don't have React experience, so please feel free to improve if needed.

[ciphrd]

Thanks for the PR! I added a few general comments for code quality.

Unfortunately, adding a feature to fx(lens) requires a bit more work than just updating here, mainly we need to:

• if it touches to the @fxhash/artist-sdk (which it does):
  • update snippet
  • update documentation about snippet
  • update related typings for the rest of the apps
• front-end normalization (we want to align the front-end as well, not just fxlens)

Currently the front-end (and other relevant packages) are sitting in our private monorepo, so you won't be able to contribute to it right now. We are planning to open-source it in a few months, but in the meantime only us can update it. Giving our current focus on integrating ETH, you may see 2-3 months until this gets implemented everywhere, but it will get there!

---

Now that's out of the way, there are also considerations in regards to the @fxhash/artist-sdk API. I believe your implementation currently suggests the following API:

$fx.params([
  {
    type: "number",
    group: "group name"
  },
  {
    type: "string",
    group: "group2 name"
  },
  //...
])

While being a straightforward implementation, this API has 2 issues for me:

• control over the order of the groups/top-level params isn't solved, and solution doesn't seem trivial
• code duplication: the group field has to be copied on every parameter in such group. This is mainly an issue for onchain projects, where bytes are a scarce resource especially on Ethereum.

Another API could be more sensible imo:

$fx.params([
  {
    type: "group",
    name: "group name",
    items: [
      {
        type: "string"
      },
      //...
    ]
  },
  {
    type: "number",
  },
  //...
])

This other API has the benefits of solving the 2 previous points, and I'm not seeing any drawback to it.

---

We'll keep you updated in here in regards to the progress made on its implementation :)

[ciphrd]

any is something we'd want to avoid in general with typescript

[ciphrd]

type T = { [key: string]: boolean }
// cleaner
type T = Record<string, boolean>

[ciphrd]

Not sure why this check was here but to reduce the area of potential failure of this PR we'd want to keep it, or explain how the check can be bypassed safely

[protozoo]

Hey there! Many thanks for reviewing my proposal and providing clear feedback.
Regarding the code quality comments, they are very welcome. As stated above, I'm not very familiar with React and only have basic understanding with Typescript, so I'm taking good note of these.

Regarding the API for defining the groups, I agree that changing the structure so that groups are defined first, and then params as 2nd level, is cleaner and avoids redundancy. Actually @fraguada suggested this yesterday in a private conversation and makes a lot of sense. I implemented it in the way I did because, as far as I understand, changing the API in that way would have required changes in other areas of the system, breaking compatibility until those were upgraded. Since the main motivation for this fork was to make the feature available to me now while working on a generative project, I thought this was the best route to go by now.

The issue with sorting the groups is "solved" in a loose way just by defining the params in the desired order (in my project I define them so that all "group 1" are defined first, then all "group 2" ones, etc... It seems from the results I got that this order is preserved when iterating the list in the UI. But yes, ideally order should be more explicit.

Finally, regarding setting a schedule for integrating this feature, I 100% understand this is not priority, specially since you guys are currently engaged in big important changes like ethereum support and more. This implementation solves my practical issue while developing generative tokens locally, so no hurry at all to get it into your pipeline. Glad to know, though, that you found it interesting and worth of your consideration 👍