Skip to content

Conversation

@UrtsiSantsi
Copy link
Contributor

No description provided.

Copy link
Contributor

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @UrtsiSantsi the code looks good to me.

However, I think the demo is broken in general.
In both Rust and Javascript, removing the code doesn't change the behaviour.
With the code it gives the following warning:

Adwaita-Critical: adw_status_page_set_child: assertion 'gtk_widget_get_parent (child) == NULL' failed

@sonnyp could you have a look since you've been the last one to touch this demo?

@UrtsiSantsi
Copy link
Contributor Author

I noticed it too, my guess is that Adwaita expects StatusPage to always have a child, but it doesn't during building from xml.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 5, 2023

Sorry for the delay.

libadwaita complains that child already has a parent.

It's correct because we have this in the Blueprint:

Adw.StatusPage {
  ...
  child: child;
}

The intention of the code was to demonstrate that the child property could be used to set the child of StatusPage.
Retrospectively, I don't think it was a good idea, it's clear that any property can be updated.

I suggest we remove all code from the "Status Page" demo and move the child inline. @UrtsiSantsi would you like to take care of it?

@UrtsiSantsi
Copy link
Contributor Author

Almost all of the demos set the status page child in the blueprint and this one does it from code, so there is some value in keeping it. Maybe we could add a stack with 2 status pages - one that sets the child in the blueprint, the other in the code?

@sonnyp
Copy link
Contributor

sonnyp commented Nov 5, 2023

In general our guideline is for the demo to be as focused as possible on the subject of the demo and avoid introducing unnecessary concepts (unless they really make sense in combination with the subject.).

Updating a property is out of scope for "Status Page".

Also, if we set the child from code only then it won't be visible in the preview on edit. Will be confusing to newcomers.

@sonnyp
Copy link
Contributor

sonnyp commented Nov 5, 2023

For reference #793

@sonnyp sonnyp deleted the rust_status_page branch November 5, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants