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

Add the ability to have circular references to Widgets (or at the very least Window) #115

Open
PhilippMDoerner opened this issue Oct 15, 2023 · 9 comments

Comments

@PhilippMDoerner
Copy link
Contributor

For Features such as CaptureWidget on SearchEntry (as discussed here ) and I think others as well, it is mandatory to be able to have circular references.

Without those implemented, the full featureset of all GTK widgets can not be provided.

I'd like to have an issue about this simply so we can keep this in mind and have somewhere to track possible progress against etc.

Though honestly, how this could possibly work is entirely beyond me.

@can-lehmann can-lehmann added this to the Owlkettle 4.0.0 milestone Oct 22, 2023
@PhilippMDoerner
Copy link
Contributor Author

PhilippMDoerner commented Nov 19, 2023

During my waiting times I'm looking into this and I'm wondering whether the first step for this ticket isn't just that we need the ability to query the gui tree we construct via the gui-dsl. Basically javascripts "getElementByTagName".

That way you could be able to fetch any Widget from a given gui tree. Imo that'd be step 1, step 2 is be able to check if a given Widget already has a corresponding GtkWidget and fetch that if it does.

Particularly the second step I have absolutely 0 idea how we could achieve that.

@can-lehmann
Copy link
Owner

React solves this issue using "refs": https://react.dev/learn/manipulating-the-dom-with-refs Something similar will work for owlkettle as well.

@can-lehmann
Copy link
Owner

The basic API might look like this:

let entry = Ref[EntryState]()
gui:
  Window:
    Box:
      Entry:
        reference = entry
      Button:
        proc clicked() =
          entry.moveCursor(0)
          entry.focus()

@PhilippMDoerner
Copy link
Contributor Author

PhilippMDoerner commented Nov 19, 2023

I never used react so I'm not fully familiar with the concept. From what I can understand, basically you create a ref first, pass that on to the widget and that "fills" the ref for you?

Edit: You were faster with the example 😄 . But yeah, given the code it's pretty much exactly that. Thanks for the explainer!

Given that BaseWidget is the easiest way to make this accessible for most widgets at once, that'd mean add a field "owlRef" or whatever to it and in the beforebuild hook, if hasOwlRef = true, then fill it with... good question. A ref to widgetState? Would make sense I guess since that nets you the entire widget + access to the associated GtkWidget

@PhilippMDoerner
Copy link
Contributor Author

I tried to play around a bit with this (basically passing WidgetState into the Ref and not making it a generic). Question:
Wouldn't a "Ref" approach like this need to sort of work like an observable in rxjs or the like?
By that I mean that you should be able to register callbacks and they fire every time the value of the Ref changes.

I've come to this conclusion mostly because I very quickly run into scenarios with the StackSwitcher/StackSidebar things where I want to use what is inside the Ref, but it does not yet have a value assigned to it, because GtkWidgets/WidgetStates only get instantiated once you insert them.

@can-lehmann
Copy link
Owner

Even though I did not originally plan this, having it be observable sounds like a good solution to the StackSwitcher issue.

@PhilippMDoerner
Copy link
Contributor Author

PhilippMDoerner commented Jun 16, 2024

As a first approach for a solution we could create a type WidgetRef = ref GtkWidget or type WidgetRef = ref WidgetState and give BaseWidget a field widgetRef that will assign the constructed GtkWidget or WidgetState to that ref.

Then you can share that around and unref it for access.

It's a bit clunky and we can see how well we can disguise that, but its the only idea I can come up with (and likely one you already thought of, but I really couldn't come up with anything better). We could look into making that type observable-ish as well. I've already implemented an observable/subject pattern in https://github.com/minamorl/rex , though using a full implementation like that might be overkill, maybe we can get away with something a lot simpler, if at all.

I assume you'd want us not depend on the lib to reduce external dependencies which I'd agree with, given we don't plan on going all in with observable.

@can-lehmann
Copy link
Owner

I think you proposed the idea of making refs observable to allow them to be used in scenarios where a widget needs to be known by another widget that is not one of its ancestors in the widget tree (e.g. Stack Switcher). For this we do not need any reactive stream logic or combinators.

I think the a ref might end up looking like this:

type
  WidgetRef*[T] = ref object
    state*: T
    observers: HashSet[proc(state: T)]

where T is some WidgetState.

@PhilippMDoerner
Copy link
Contributor Author

PhilippMDoerner commented Jun 16, 2024

I don't think that's possible. The main reason being that you'd want to implement it roughly like this:

#widgetutils.nim
type
  StateRef*[T: WidgetState] = ref object
    state: T
    observers: HashSet[proc(state: T)]

proc hasRef*[T: WidgetState](stateRef: StateRef[T]): bool = not stateRef.state.isNil()

proc newRef*[T: WidgetState](): StateRef[T] = 
  StateRef[T](observers: initHashSet[proc(state: T)]())

proc unwrap*[T: WidgetState](stateRef: StateRef[T]): T =
  stateRef.state

proc setRef*[T: WidgetState](stateRef: StateRef[T], state: T) =
  stateRef.state = state
  for observer in stateRef.observers:
    observer(stateRef.state)

proc widget*[T: WidgetState](stateRef: StateRef[T]): Option[GtkWidget] = 
  when T is Renderable: # This doesn't work so far for some reason 
    some stateRef.state.internalWidget
  else:
    none(GtkWidget)

proc subscribe*[T: WidgetState](stateRef: StateRef[T], observer: proc(state: T)) =
  stateRef.observers.incl(observer)

proc unsubscribe*[T: WidgetState](stateRef: StateRef[T], observer: proc(state: T)) =
  stateRef.observers.excl(observer)

#widgets.nim
renderable Box of BaseWidget:
  ...
  stateRef: StateRef[BoxWidgetState]
        
  hooks:
    ...
    afterBuild:
      if not state.stateRef.isNil():
        state.stateRef.setRef(state)

Now this is a problem, BoxState doesn't exist yet as it has not yet been created so it can't be used in a type-declaration.
Therefore it'd have to be Renderable so you can have access to at least GtkWidget, which is the key.

Edit:
Or you generate this field inside the DSL, as well as the code for the afterBuild hook

Edit2:
I think generating this as part of the DSL is the way to go after mulling it over a bit.

PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 19, 2024
This introduces the StateRef type which is a ref-type containing a reference to a Widget.
It is used for the pseudo field "stateRef" for each Widget, which for any given Widget will contain a reference to said widget.
This allows accessing references within Widgets and passing them back and forth.

It is reactive, to allow subscribing to it and getting updated every time the reference changes.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 19, 2024
It should not be filled in a given playground, therefore it should not show up in there in the first place.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 19, 2024
The playground was not checking for nil if a value on a state was a ref-type.
That lead to seg faults.
It now does those checks.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 19, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 21, 2024
Previously the stateRef got the observer unsubscribed and resubscribed every update.
That shouldn't happen, we only want to do that when the stateRef actually change.
Therefore we now check first for changes in the ref.

That uncovered that we need to also do the initial subscription.
This is now handled in the build hook for this field.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
Modifies both parsing the gui-DSL into Node-types as well as generating code from those Node-types.

First it allows parsing expressions such as "<Widget> as <someStateRefVariable>".
This is supposed to express an assignment of <someStateRefVariable>
to the "valStateRef" Field on the <Widget> instance.
The variable gets stored in the "widgetRefVar" field on "Node".
There it is optional, as expressions such as "<Widget>:" without the "as <someStateRefVariable>" syntax will not contain assignments.

The "<Widget> as <someStateRefVariable>" syntax creates an infix expression which is part of nnkCallKinds.
Therefore in parseGui I removed it from the nnkCallKinds group.

In the code-generation step, when generating a NodeWidget there now is a small check if the gui-DSL contained such an assignment. If so, the variable <someStateRefVariable> gets assigned to the Widgets "valStateRef" field.

This is functionally equivalent to just having this syntax:
Widget:
  stateRef = <someStateRefVariable>

But is more readable.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
The Syntax "<Widget>() as <someStateRefVariable>" did not work.
This was due to a parsing-oversight in parseGui.
This PR corrects that.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
Check for ident("as") by using eqIdent instead of comparing kind and stringified value.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
When using pragmas, specifically adders,
the "as" syntax does not function gracefully.
It will break as the infix-parsing-section of parseGUI
is unfamiliar on how to deal with pragmas.

This change fixes that.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
The stateRef field is a custom field that does not stem from assignment anyway.
Therefore it is not necessary to follow the "val<Field>"/"has<Field>" convention
for fields.
We know it is a ref-type, just do a nil-check instead.
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
PhilippMDoerner added a commit to PhilippMDoerner/owlkettle that referenced this issue Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants