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

GTK4/libadwaita port #464

Closed
wants to merge 19 commits into from

Conversation

togetherwithasteria
Copy link
Contributor

@togetherwithasteria togetherwithasteria commented Aug 3, 2022

Hiiiii!!! <3

This PR solves #418's author's request for a GTK4 port

But at the moment, it's not yet complete. The Flatseal window will be completely empty for now!!! Fixed nowww!!!

  • Fix widgets not showing up!
    image
    It seems many widgets' visible properties are set false. Turning them on manually trough the GTK inspector results in this screenshot!!!
    image
  • Text boxes won't allow writing!!
    • The search text box
    • The preferences text boxes
  • Switch to an alternative for webkitGTK. WebkitGTK for GTK4 is not ready yet for now!!! But considering this is just for the help menu, we could just open the HTML file trough Xdg Open?

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
@togetherwithasteria
Copy link
Contributor Author

image
Yayy!!!

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
I think this functionality is baked inside of the widget already?

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
I missed a lot of them in the last commit about this 😭😭😭

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
…this.rows inside of that loop!!!

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
Copy link
Owner

@tchx84 tchx84 left a comment

Choose a reason for hiding this comment

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

@lovelyyfiaaa woah!! <3

Ok, OK, this is going to take a few iterations... and to make this digestible for me please 🙏

  1. Rebase on top of current master.
  2. Let's leave linting preferences for later, and make this PR exclusively about GTK4/Adw. Nothing more nothing less.

I really appreciate this 😍

@togetherwithasteria
Copy link
Contributor Author

togetherwithasteria commented Aug 6, 2022

Let's leave linting preferences for later, and make this PR exclusively about GTK4/Adw. Nothing more nothing less.

Sorry, but apparently my code editor was configured to automatically format things on save!!!

@togetherwithasteria
Copy link
Contributor Author

@tchx84 Can I ask you some questions?

  1. There seems to be an "actionBar" that wasn't visible in the GTK3 Flatseal, but it exists!
    In my port it was visible, though I removed them because it wasn't intended to be there in the GTK3 version, it seemed. Should I remove them or enable them?
  2. Emmm, since WebkitGTK for GTK4 isn't ready yet, would you want the Documentation button to open the HTML file in the default browser instead

@tchx84
Copy link
Owner

tchx84 commented Aug 8, 2022

@tchx84 Can I ask you some questions?

Absolutely, please keep asking.

  1. There seems to be an "actionBar" that wasn't visible in the GTK3 Flatseal, but it exists!
    In my port it was visible, though I removed them because it wasn't intended to be there in the GTK3 version, it seemed. Should I remove them or enable them?

It's should only be visible when the window's width shrinks pass a threshold (try it on the GTK3 version to see the behavior).

  1. Emmm, since WebkitGTK for GTK4 isn't ready yet, would you want the Documentation button to open the HTML file in the default browser instead

Given that constraint, that's probably a fair approach.

Let's not aim for perfect yet, let's just see far this can go.

Signed-off-by: Nefo Fortressia <nefothingy@hotmail.com>
@togetherwithasteria
Copy link
Contributor Author

It's should only be visible when the window's width shrinks pass a threshold (try it on the GTK3 version to see the behavior).

Ah, I see!!
Screenshot of a portrait view of Flatseal showing the settings for Darkbar

@JanDeDinoMan
Copy link

Text boxes won't allow writing!!

After a bit of investigating this is because in src/widgets/window.ui all widgets have the property can-focus set to False. Setting these to True or removing the the property (since they default to True) will resolve this problem. As seen here: JanDeDinoMan@ca6a44f

@togetherwithasteria
Copy link
Contributor Author

Text boxes won't allow writing!!

After a bit of investigating this is because in src/widgets/window.ui all widgets have the property can-focus set to False. Setting these to True or removing the the property (since they default to True) will resolve this problem. As seen here: JanDeDinoMan@ca6a44f

Okayyy

Sorry, I haven't been able to work on this again for quite some time since I went busy. I'll start working again on this PR.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

These buttons should be flat

image

The reason this happens is because they are setting a GtkImage as their child instead of using the button icon-name property.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

This GtkListBox

image

Should use the sidebar css class.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The reset button

image

should be on the right of the headerbar, one has to use <child type="end"> to achieve this.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

Padding and positions are off, compare

image

image

also some text requires the dim-label class here. Note that the current way of setting the font type is via css classes, for example the title can use title-1 or title-2, and the subtitle can use the heading class. You can see the available classes with https://flathub.org/apps/details/org.gnome.design.Typography

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

Every button in a row should use the flat css class

image

note that the margins on the bottom of the rows do not coincide with the margin on the top. This would be easier to acomplish with AdwActionRow.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

This button should be flat and have a 6px separation with the switch. Note that toggling the switch twice won't hide the "clear" button. I dont know if the later is an issue in stable.

image

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The about dialog is broken for some reason

image

Please use AdwAboutDialog instead. I think the issue is that the dialog is a subclass of GtkAboutDialog which is final, i.e. not subclassable.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The controllers appear twice in "mobile mode" , and the "back" button appears incorrectly as insensitive.

image

the controls on the bottom don't hide themselves when you increase the size of the window too.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The search bar requires some tweaks

image

Ideally it should be a GtkSearchBar containing a GtkSearchEntry, this handles input (which is not working correctly), right now it is using a revealer.

See https://docs.gtk.org/gtk4/method.SearchBar.set_key_capture_widget.html.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The environment variables looks odd

image

I am not quite sure how to fix it, maybe using AdwEntryRow? This requires design I think.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

I see uses of <property name="icon_size">6</property>, this takes an enum https://docs.gtk.org/gtk4/enum.IconSize.html instead of numerical values.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

I don't know if this is supported in gtk4

@binding-set WindowBindings {
  bind "<Primary>q" { "close" () };
  bind "<Primary>f" { "find" () };
}

window {
  -gtk-key-bindings: WindowBindings;
}

Please use https://docs.gtk.org/gtk4/method.Application.set_accels_for_action.html

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

The enviaroment variables throw an error

(com.github.tchx84.Flatseal:2): Gjs-CRITICAL **: 20:50:01.914: JS ERROR: TypeError: row.destroy is not a function
_remove@resource:///com/github/tchx84/Flatseal/js/widgets/pathsViewer.js:56:11
_remove@resource:///com/github/tchx84/Flatseal/js/widgets/variableRow.js:67:14
main@resource:///com/github/tchx84/Flatseal/js/main.js:38:24
run@resource:///org/gnome/gjs/modules/script/package.js:206:19
@/app/bin/com.github.tchx84.Flatseal:9:17

when adding/removing them.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

I see some errors of the like

(com.github.tchx84.Flatseal:2): Gtk-WARNING **: 20:51:43.759: Finalizing GtkInspectorLogs 0x7f9c78004150, but it still has children left:
 - GtkBox...

Every widget that uses set_parent() should use unparent() in its dispose vfunc. I think this actually comes from setting a child widget in the UI file to a widget that does not support this.

@A6GibKm
Copy link
Contributor

A6GibKm commented Nov 3, 2022

Despite the large number of issues, the port looks very good and promising. Thanks! Ill do a more careful review of the code once it leaves Draft status.

A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Mar 31, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Mar 31, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Mar 31, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 1, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 2, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 2, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 5, 2023
This fixes regression issues with focus, as per

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
A6GibKm pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 5, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.

tchx84#464 (comment)
Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
@tchx84
Copy link
Owner

tchx84 commented Apr 5, 2023

This work is being continued here #515 , so I am closing this.

Thanks a lot @natasria for working on this.

@tchx84 tchx84 closed this Apr 5, 2023
@A6GibKm
Copy link
Contributor

A6GibKm commented Apr 5, 2023

Thanks @natasria !

tchx84 pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 5, 2023
This fixes regression issues with focus, as per
tchx84#464 (comment)

Closes tchx84#517

Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
tchx84 pushed a commit to A6GibKm/Flatseal that referenced this pull request Apr 5, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.
tchx84#464 (comment)

Closes tchx84#517

Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
tchx84 pushed a commit that referenced this pull request Apr 5, 2023
This fixes regression issues with focus, as per
#464 (comment)

Closes #517

Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
tchx84 pushed a commit that referenced this pull request Apr 5, 2023
Previously, the actionbar will always be visible.
It should have not been that.
Instead, it should only appear when the screen is small.
#464 (comment)

Closes #517

Signed-off-by: Fiana Fortressia <fortressnordlys@outlook.com>
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

Successfully merging this pull request may close these issues.

None yet

4 participants