Add autoform to all the widgets

[PhilippMDoerner]

Basically a "continuation" of #93 . This PR simply adds autoform to all widget examples under /examples/widgets.
Again, the only files changed here are actually widgets.nim and the various example files, anything else stems from either #90 or #94.

This should only be merged (and honestly also only reviewed) after #94 was merged as it directly depends upon that PR.

The exact examples in total that autoform was added to encompass thusly:

• Scale (From PR #93 add form generator #94 )
• Calendar
• Popover Menu
• Radio Group
• Drop Down
• Picture
• Text View

Beyond just adding autoform, I also added the fields of the BaseWidget to those examples so that users can be made more aware that they also have access to those and can see their effects on the different widgets.

Further this PR also fixes a small bug with Radio Group:
It was missing a property hook for its orient field.
This PR adds the missing property hook in widgets.nim, which enables RadioGroup to change its orientation at runtime, which previously wasn't possible.

[PhilippMDoerner]

Alrighty, the recent changes should've added the new Popover version to the examples I've listed for individual widgets.
Also added the MIT License to autoform.nim as you usually did for all the other files in owlkettle.

I also did a small overhaul, mostly to "simplify"/"unify" forms for types like seq[string] etc.

If you have a field of type MyCustomType, you must implement proc toFormField(state: auto, fieldName: static string, typ: typedesc[MyCustomType]): Widget =.

If you have a field of type seq[MyCustomType], you must now implement proc toListFormField(state: auto, fieldName: static string, index: int, typ: typedesc[MyCustomType]): Widget =

That way you only need to worry about making a form for one entry, not for an entire list of them.
The way it works is that basically there's already a toFormField defined for seq[T] and that one just calls toListFormField to generate the individual rows, puts it all into an ExpanderRow and provides a "plus" button to add new entries.
I couldn't figure out how to easily just add a Delete Button for every row, but I think that's fine.

If you have better idea for wording, shoot it out, I'll happily improve upon the meh names I landed on for these procs. Naming is hard.

Now with that out of the way, I have the following questions:

1. Should the form and the entire concept remain being called "autoform"? It was literally just the first name that popped, another one of those "If you got a better name, feel free to suggest one, I'll be happy to rename module procs and docs accordingly"
2. I noticed that while going over the widgets: Not every widget has an example like this, right? Immediate things would be stuff like "Button" or "Label". Would it make sense to add more widget examples? If yes, should I add them in this PR or should we first merge this and then I make a new PR with more widget example applications ? (The goal being 1 example app per widget in widgets.nim and adw.nim)

[can-lehmann]

Should the form and the entire concept remain being called "autoform"? It was literally just the first name that popped, another one of those "If you got a better name, feel free to suggest one, I'll be happy to rename module procs and docs accordingly"

We could already call it playground in preparation for potential future applications.

I noticed that while going over the widgets: Not every widget has an example like this, right? Immediate things would be stuff like "Button" or "Label". Would it make sense to add more widget examples? If yes, should I add them in this PR or should we first merge this and then I make a new PR with more widget example applications ? (The goal being 1 example app per widget in widgets.nim and adw.nim)

I decided to only create examples for the more complex widgets pretty early on, since I trust that users should be able to figure out how to use the basic widgets like Button, Label, ... from the docs, tutorial and other examples. I don't think that the added maintenance work for examples for every widget is worth it.

[can-lehmann]

I played with some of the examples and I very much like it! I think that the new design works quite well.

A few general notes:

• Currently all examples have two header bars, you need to add the HeaderBars using addTitlebar.
• All buttons within a HeaderBar should have the style ButtonFlat (as in basically all other Adwaita apps, probably also specified in the HIG)
• I would show the currently selected date for the DateTime form in the description of the ActionRow, which makes it take up less space horizontally.
• There is lots of code duplication between toListFormField and toFormField, this should be reduced (it might be possible to just remove the difference between the two procs entirely?).

[PhilippMDoerner]

Punkt:

1. Done
2. To be done
3. Done
4. That one we run into a conceptual issue.

Particularly for 4: The only reason these are separate is because in toFormField i need to generate expressions along the lines of state.myField, in toListFormField I need to generate expressions along the lines of state.myField[index].
That is the only reason these two functions are separate.

I want very much to act directly on state because I know that is a ref that is shared between the main widget in the example as well as every proc in autoform that gets passed said state.
Since they all act on the same state object in heap, the manipulation from autoform widgets can affect the values in the renderable.

I can not assume that every field on the state is a ref type.
That means that a field might be a value type and that I might get copies when passing a field by value to a proc.
That'd in turn would lead to the autoform widgets modifying their own copies of a values that originally came from state without modifying the actual thing on state.

I'll launch an attempt at using fieldValue: var <Type of fieldvalue> instead of state: auto, fieldName: static string, typ: <Type of fieldValue> and state: auto, fieldName: static string, typ: <Type of fieldvalue> in the hopes that this'll get nim to use references under the hood. However, I make no guarantees and no promises.

[PhilippMDoerner]

I can't seem to find any approach that allows me to unify toFormField and toListFormField and get rid of the code duplication.

The approach of "just pass the fieldValue as a mutable parameter var T doesn't work because you can't capture it in an event-handler if it's a value-type:

proc toFormField[T: SomeNumber](fieldName: static string, fieldValue: var T): Widget =
  ## Provides a form field for any number type
  return gui:
    ActionRow:
      title = fieldName
      FormulaEntry() {.addSuffix.}:
        value = fieldValue.float
        xAlign = 1.0
        maxWidth = 8
        proc changed(value: float) =
          fieldValue = value.T

'fieldValue' is of type <var float> which cannot be captured as it would violate memory safety, declared here: /home/philipp/dev/owlkettle/owlkettle/autoform.nim(35, 33); using '-d:nimNoLentIterators' helps in some cases. Consider using a <ref var float> which can be captured.

Since you can't assume that every type on a state is a ref-type, you must capture the appState access the fields on that directly.
So your only options are trying to generate that code in a more elegant manner.

I did various attempts centered around templates for that, the most recent one being to turn the gui block into a template like this:

template stringField(fieldName: string, fieldExpression: untyped): Widget =
  ## Provides a form field for string
  gui:
    ActionRow:
      title = fieldName & "I am from the trees"
      Entry() {.addSuffix.}:
        text = fieldExpression
        proc changed(text: string) =
          fieldExpression = text

proc toFormField(state: auto, fieldName: static string, typ: typedesc[string]): Widget =
  ## Provides a form field for string
  return stringField(fieldName):
    state.getField(fieldName)

proc toListFormField(state: auto, fieldName: static string, index: int, typ: typedesc[string]): Widget =
  ## Provides a form to display a single entry of type `string` in a list of `string` entries.
  return stringField(fieldName & $index):
    state.getField(fieldName)[index]

All of them blow up with the error:

stack trace: (most recent call last)
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(293, 17) gui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(111, 36) parseGui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(94, 11) parseGui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(111, 36) parseGui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(94, 11) parseGui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(111, 36) parseGui
/home/philipp/dev/owlkettle/owlkettle/guidsl.nim(113, 11) parseGui
/home/philipp/.choosenim/toolchains/nim-2.0.0/lib/std/assertions.nim(41, 14) failedAssertImpl
/home/philipp/.choosenim/toolchains/nim-2.0.0/lib/std/assertions.nim(36, 13) raiseAssert
/home/philipp/.choosenim/toolchains/nim-2.0.0/lib/system/fatal.nim(53, 5) sysFatal
/home/philipp/dev/owlkettle/examples/widgets/drop_down.nim(38, 12) template/generic instantiation of `gui` from here
/home/philipp/dev/owlkettle/examples/widgets/drop_down.nim(46, 19) template/generic instantiation of `toAutoFormMenu` from here
/home/philipp/dev/owlkettle/owlkettle/autoform.nim(321, 27) template/generic instantiation of `toAutoFormMenu` from here
/home/philipp/dev/owlkettle/owlkettle/autoform.nim(302, 28) template/generic instantiation of `toFormField` from here
/home/philipp/dev/owlkettle/owlkettle/autoform.nim(277, 38) template/generic instantiation of `toListFormField` from here
/home/philipp/dev/owlkettle/owlkettle/autoform.nim(184, 21) template/generic instantiation of `stringField` from here
/home/philipp/.choosenim/toolchains/nim-2.0.0/lib/system/fatal.nim(53, 5) Error: unhandled exception: /home/philipp/dev/owlkettle/owlkettle/guidsl.nim(113, 7) `node[0].isName`  [AssertionDefect]

Beef had some approach regarding turning toFormField procs in general into templates, but that ran into the same error.
He mentioned something about the gui macro being an untyped macro causing some limitations there (?).

Over all, I got nothing regarding deduplicating this.

That is, outside of using addr to create pointers to the fields on AppState (even if they're value-types), which is an approach so cursed that I explored whether it may be feasible in theory but did not attempt.

[PhilippMDoerner]

All buttons within a HeaderBar should have the style ButtonFlat (as in basically all other Adwaita apps, probably also specified in the HIG)

Updated this just now:

• Changed the autoform menubutton to be flat
• Changed the menubutton in the examples for:
  • popover menu
  • text_view and
  • almost all the headerbar button in the picture example into Flat buttons. I did not do that for the "open" button on picture because that overwrites its "ButtonSuggested" styling and I assumed that was important.

Edit:
I also did the rename to "playground".
At this point I think I've addressed all the things I can address (I don't think I overlooked any of your feedback?) and the one thing I couldn't address (removing code-duplication in the now renamed playground.nim) is one I've got no solution for.
If you've got any other approaches for generating the form I'm all ears, otherwise I think this PR is finished-ish?

[can-lehmann]

ignoreFields is not necessary

is sizeRequest necessary?

[PhilippMDoerner]

I'd like to keep the sizeRequest as the default size of the menu is more suited to an amount of fields like Scale has.
Thus (in my opinion) it looks nicer if the height is just enough to cover the fields.
With the default values it looks like this:

[can-lehmann]

That is, outside of using addr to create pointers to the fields on AppState (even if they're value-types), which is an approach so cursed that I explored whether it may be feasible in theory but did not attempt.

That is actually the way to go here from what I can see. However, since this PR is currently blocking your other widgets, I would be fine with merging this as is (provided the things in the review above are fixed) and going over playground/autoform again at a later stage & refactoring it to use ptr (or at least attempt to).

[PhilippMDoerner]

I addressed the remaining points and I'm happy to tackle the refactoring PR with addr as the next one.
Or rather I can combine those, as you mentioned preferring larger PRs?
Make a combo PR for both #85, #86 and add the playground refactor on top of that?

[can-lehmann]

I must say that I am really happy with this PR at this point. Good work 😄 !

I addressed the remaining points and I'm happy to tackle the refactoring PR with addr as the next one.
Or rather I can combine those, as you mentioned preferring larger PRs?
Make a combo PR for both #85, #86 and add the playground refactor on top of that?

The point about larger PRs was mainly about when they actively influence each other. E.g every time you changed something in #94, this impacted this PR, so thats why I prefer one single, larger PR in that case. If the features are independent, having separate PRs is of course nicer, however, I would also be fine with a single larger PR if that is less work for you. The main issue I have is just when the features actively depend on each other, which makes reviewing quite difficult when they are spread across multiple PRs.

If I remember correctly, you already implemented #85 and #86 in large parts. Given that the (at least planned) release date for 3.0.0 approaches quickly, it would make sense to merge them before then.

[can-lehmann]

Looks like the already paid off. While playing with the examples, I found a pretty stupid bug in the Calendar widget: 8e915e3