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

Allow gtk modifiers to be expressed as css attributes. #345

Merged

Conversation

mortenbekditlevsen
Copy link
Contributor

Apply this to background modifier and added rectangle overlay (border) modifier.

I don't know if it's the best API to have to conform to both WidgetModifier and WidgetAttributeModifier, but it does the trick...

@mortenbekditlevsen
Copy link
Contributor Author

Next up: allow attributes to be merged - and allow specifying something like isOrderDependent like in the DOMViewModifier.

@MaxDesiatov MaxDesiatov added the GTK renderer Extending GTK support label Dec 27, 2020
@MaxDesiatov
Copy link
Collaborator

I didn't know that GTK supports this. Do you think we could share any of this code with the HTML and DOM renderers?

@MaxDesiatov MaxDesiatov requested review from a team December 27, 2020 17:11
@mortenbekditlevsen
Copy link
Contributor Author

mortenbekditlevsen commented Dec 28, 2020

I'm by no means an expert, but gtk has a css based theming engine, so a user can place style sheets and icons somewhere in their home directory in order to have all of their gtk apps themed.
I gave the style provider the lowest priority (as suggested by documentation), so even those things should be user overridable.

As for sharing code with the HTML/DOM renderers, I don't think there's much to gain.
Only a limited subset of css styles are supported and some only works on special widgets.
For instance I tried adding padding support, but this doesn't work directly on a label widget. It works on a frame widget, so a workaround would be to sometimes wrap the widget in another widget depending on the situation.

With regards to performance, it would probably be better to implement a custom gtk 'style provider' rather than parsing css snippets all the time, but that also might be a bit of work, so I chose the existing css style provider for now. It makes it easy to play with the possibilities at least! :-)

Copy link
Collaborator

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

I didn't test it, but it seems legit 👍

@MaxDesiatov
Copy link
Collaborator

I wonder if we could integrate the SnapshotTesting library at some point to verify that everything is rendered as we expect?

@MaxDesiatov
Copy link
Collaborator

@mortenbekditlevsen I've sent you an invitation to the GTK Tokamak team, but it's no longer listed in the pending invitations list for some reason. Did you receive it or did it expire then?

@mortenbekditlevsen
Copy link
Contributor Author

@MaxDesiatov Thanks! 😊 I didn't receive an email about it.

@MaxDesiatov
Copy link
Collaborator

Here's yet another attempt 🙂
Screenshot 2020-12-28 at 17 21 03

@mortenbekditlevsen
Copy link
Contributor Author

mortenbekditlevsen commented Dec 28, 2020

The snapshot testing library is built to be extended and I can see in the gtk documentation that there is an offscreen window 'widget' exactly for offscreen rendering purposes. https://developer.gnome.org/gtk3/stable/GtkOffscreenWindow.html

@MaxDesiatov
Copy link
Collaborator

That makes sense. At least when rendering for it is stable enough, we should strongly consider having some tests that cover it (and that's why snapshot testing came to mind) to prevent regressions.

@mortenbekditlevsen
Copy link
Contributor Author

Thanks, I completely missed your message about the invitation, but I just saw it and accepted.
Thanks so much.

I'm currently experimenting with Shapes using a drawing area widget and importing GDK in order to get access to the drawing routines.

I have a few thoughts about how to implement CGPath on non-Darwin machines - inspired by the CGPath.apply function that exposes some of the internals of CGPath.

@MaxDesiatov
Copy link
Collaborator

Excited about your progress with CGPath!

Feel free to merge your PRs after they're approved, you may wait for a couple days for more people to approve before you merge. But this one has been in the review queue for long enough, has already at least one approval, and seems trivial enough to be merged already 🙂

@mortenbekditlevsen
Copy link
Contributor Author

Excellent! And the check-labels failing check is ok?

Regarding CGPath I failed to realize that SwiftUI.Path is basically the same API. But I'll try to add support for paths using path segments instead of the other storage shortcuts.

@MaxDesiatov
Copy link
Collaborator

MaxDesiatov commented Dec 31, 2020

The check-labels check is displayed as passing for me. It may be displayed as failing in the mobile GitHub app, but it's a bug, as it doesn't remove the old status of the check after you assign a label on a PR.

Screenshot 2020-12-31 at 20 31 25

@mortenbekditlevsen mortenbekditlevsen merged commit a97a05f into TokamakUI:main Dec 31, 2020
@mortenbekditlevsen mortenbekditlevsen deleted the gtk-attribute-modifier branch December 31, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GTK renderer Extending GTK support
Development

Successfully merging this pull request may close these issues.

None yet

2 participants