can-lehmann / owlkettle

A declarative user interface framework based on GTK 4
https://can-lehmann.github.io/owlkettle/README
MIT License
367 stars 14 forks source link

Fix or document the usage problems with non-PreferenceRow based Widgets with ExpanderRow #135

Open PhilippMDoerner opened 7 months ago

PhilippMDoerner commented 7 months ago

For reference: This is a follow up issue that arose during PR #131

We noticed issues when adding Widgets such as "Box" via addRow to an ExpanderRow Widget. That caused runtime issues when trying to remove said Widget. Here a minimal example to reproduce said issue:

import owlkettle, owlkettle/adw

viewable App:
  numbers: seq[int] = @[1,2,3]

method view(app: AppState): Widget =
  result = gui:
    Window:
      ExpanderRow:
        title = "Expander Row"

        for index, value in app.numbers:
          Box {.addRow.}:
            Button():
              icon = "user-trash-symbolic"
              style = [ButtonDestructive]
              proc clicked() =
                app.numbers.delete(index)

adw.brew(gui(App()))

Steps to reproduce:

  1. Compile the example
  2. Open the expander row
  3. Click on the delete button - It will not remove the row and instead show this GTK warning in the terminal: Gtk-WARNING **: 19:02:24.292: Tried to remove non-child 0x556e76e45c30

During our discord chats you figured that this piece of GTK code explains said issue - In that they use different adder-procs based on PreferencesRow Widgets.

I don't really understand how that explains the problem, but it did solve it in that using PreferenceRow based Widgets instead (e.g. ActionRow, ComboRow etc.) solves this problem.

We should either fix this (I honestly don't fully understand how we would do such a thing because I don't understand from the beginning what the problem is) or at the absolute minimum we should add docs to ExpanderRow to only use PreferenceRow based Widgets with it.

PhilippMDoerner commented 7 months ago

Sidenote, I've expanded upon the minimal example a bit. If you add to widgetutils.nim the following:

You can see where the individual GTKWidgets get fully added and removed. Therefore you can see that this is an erroneous error message because you can see the widget getting added to ExpanderRow.

Example logs:

Fully new add: 000055ACA5F99E70
Fully new add: 000055ACA5FF5360
Fully new add: 000055ACA5F57C30 # This is the exact same pointer that later gets removed!!!
Remove Finally: 000055ACA5F57C30

(process:1257091): Gtk-WARNING **: 09:29:23.661: Tried to remove non-child 0x55aca5f57c30

I am currently assuming that maybe this is a lack of documentation on the GTK part that you're not intended to use Box Widgets (or any widget that isn't a PreferenceRow) with ExpanderRow.

I have opened a thread on gnome's discourse forum regarding this. Maybe this will shine some light on the situation

PhilippMDoerner commented 7 months ago

I'm getting a bit closer here. This might actually be a bug in Adwaita and not owlkettle (see latest posts in the forum thread).

To test this I made a custom widget with expander-row and did creating the expanderRow Widget, creating the box, adding it and removing it directly in the beforeBuild hook:

#adw.nim
renderable DummyExpander of PreferencesRow:

  hooks:
    beforeBuild:
      state.internalWidget = adw_expander_row_new()
      let newBox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 0.cint)
      echo "NewBox Pointer: ", newBox.pointer.repr
      adw_expander_row_add_row(state.internalWidget, newBox)
      adw_expander_row_remove(state.internalWidget, newBox)

# example.nim
import owlkettle, owlkettle/adw

viewable App:
  discard
method view(app: AppState): Widget =
  result = gui:
    Window:
      DummyExpander()

adw.brew(gui(App()))

This also produces the issue. I currently see this as a strong indicator with the issue not lying with owlkettle here.

PhilippMDoerner commented 7 months ago

Filed a bugreport with the adwaita repository on gnome's gitlab: https://gitlab.gnome.org/GNOME/libadwaita/-/issues/758