Gtk4 port part 2 #515

A6GibKm · 2023-03-31T12:52:53Z

This is a continuation of #464

Missing:

FlatsealPermissionEntryRow: Needs to be a expander row with EntryRows as children (implemented it in another fashion)
DocView: The background needs a new color
ShowDetails opens software twice for some reason (Pre-existing issue)
Markup error somewhere
Port notification to AdwToast
Entry Completions are probably borked
Close buttons on rows inherit the accent_color, this is wrong

tchx84 · 2023-03-31T13:20:00Z

@A6GibKm awesome, I'll give it a try today and come back.

A6GibKm · 2023-03-31T14:10:43Z

DocView: The background needs a new color

Where would one set this? I see there is a stylesheet in index.html, but it seems highly non-editable.

tchx84 · 2023-03-31T14:15:39Z

DocView: The background needs a new color

Where would one set this? I see there is a stylesheet in index.html, but it seems highly non-editable.

This is generated from Apostrophe's export MD to HTML option, plus a hack I added to remove the embedded fonts. I basically run this https://gitlab.gnome.org/tchx84/apostrophe/-/tree/flatseal .

If it's too troublesome we can leave that for later.

A6GibKm · 2023-03-31T16:01:00Z

Might I ask what code reveals the in-app notification? I cannot find it.

tchx84 · 2023-03-31T17:21:50Z

Might I ask what code reveals the in-app notification? I cannot find it.

It's the widget itself connecting to the permissions model signals, see https://github.com/tchx84/Flatseal/blob/master/src/widgets/undoPopup.js#L42

tchx84

It looks quite good, here's a quick dump of a few details I noticed:

keyboard: Navigation gets stuck or certain UI elements can't be focused, e.g, use arrows or tab to navigate trough the UI and it will get stuck on apps list or permissions list.
keyboard: Activation of the main menu (F10) doesn't work e.g, pressing F10 won't open the menu.
keyboard: Menu items mnemonics don't show up e.g, pressing ALT while menu it's open, won't make the mnemonics show up.
keyboard: Activation of apps search (CTRL+F) doesn't work e.g, only seems to work if the search button is focused.
window: When in "compact" mode, can't swipe back from the permissions view to the apps view.
window: The lower "show details and reset" menu would stay around after the window is back at full size, e.g. switch the window to compact mode, go to the permissions view and then double click the header bar to maximize back to full size.
permissions: The "other files" section doesn't seem to reset properly e.g, select Flatsealon the apps list, check the "other files" section, now re-select Flatseal multiple times on the apps list, and a lot of duplicate entries will appear on the "other files" section.
permissions: In addition to the issue mentioned above, these duplicates seem to persist, switching to another app and back doesn't seem to fix it. This doesn't seem to happen at the model level (reset button is kept inactive), so not entirely sure what's going on.
permissions: In addition to the above, the issue seems to happen to other similar widgets e.g. dbus, variables, persist, etc.
documentation window: Window title shows app id not the app name.
documentation window: keyboard shortcuts don't seem to work e.g, CTRL+F won't activate search entry, CTRL+Q won't close the window.

src/widgets/aboutDialog.js

A6GibKm · 2023-04-01T07:47:50Z

keyboard: Navigation gets stuck or certain UI elements can't be focused, e.g, use arrows or tab to navigate trough the UI and it will get stuck on apps list or permissions list.

Could you be more precise on which elements please? Here it works, but there are a few GTK 4 rough edges, like going from the listbox on the left to the header bar. Or going from left panel to right panel using TAB only.

You have no clue how helpful this review was. I found multiple things that were broken in more apps and simple fixes for them.

keyboard: Activation of the main menu (F10) doesn't work e.g, pressing F10 won't open the menu.

fixed, was it working in gtk3? if so how?

keyboard: Menu items mnemonics don't show up e.g, pressing ALT while menu it's open, won't make the mnemonics show up.

GTK 4 quirk, they appear when starting keynav inside of them, ill try to re-open upstream the issue as it is indeed weird.

window: When in "compact" mode, can't swipe back from the permissions view to the apps view.

What I can reproduce here, is that swiping back goes to the separator (which looks like a grey page) instead of the list of apps, I fixed this. Could you confirm?

permissions: The "other files" section doesn't seem to reset properly e.g, select Flatseal on the apps list, check the "other files" section, now re-select Flatseal multiple times on the apps list, and a lot of duplicate entries will appear on the "other files" section.

This and related items should be fixed in a later commit, try fetching latest commit and please confirm if the issue was indeed fixed.

documentation window: Window title shows app id not the app name.

Fixed, incidentally, the app name was the app id, I fixed this too.

documentation window: keyboard shortcuts don't seem to work e.g, CTRL+F won't activate search entry, CTRL+Q won't close the window.

Fixed

EDIT:

window: The lower "show details and reset" menu would stay around after the window is back at full size, e.g. switch the window to compact mode, go to the permissions view and then double click the header bar to maximize back to full size.

Fixed

A6GibKm · 2023-04-01T08:57:07Z

Also CI is broken, no idea how to fix this one as it uses an image. Probably we can use https://github.com/bilelmoussaoui/ashpd/tree/master/.github/workflows.

EDIT: Used just that and CI passes. I have yet to try the bundle.

A6GibKm · 2023-04-01T09:31:11Z

I found the following thing, the about dialog has a list of translators, what we usually do is to add a

    <property name="translator_credits" translatable="yes" comments="Add your name to the translator credits list">translator-credits</property>

each translation adds the credit for the specific translators in use instead of having a list of all translations. Do you want to implement this change in this PR?

A6GibKm · 2023-04-01T13:05:38Z

Only things missing from my side are

Using #242424 (dark mode) and #fafafa (light mode), which correspond to @window_bg_color, in the document viewer. This is out of scope for the PR. Probably also check @accent_color for links. It might be ideal if the css file is in its own file so its easier to edit.
Checking if there are further key navigation issue that are not intrinsic GTK 4 issues
Libadwaita allows setting accent colors different from blue, are you interested in doing so?

See https://gnome.pages.gitlab.gnome.org/libadwaita/doc/main/named-colors.html

tchx84 · 2023-04-01T14:22:18Z

.github/workflows/CI.yml

+        bundle: "flatseal.flatpak"
+        manifest-path: "com.github.tchx84.Flatseal.json"
+        run-tests: "true"
+        cache-key: flatpak-builder-${{ github.sha }}


The tests are "passing" because jasmine-gjs is not included here, and therefore the actual tests are not running.

I am working on this here #514 it has it owns issues. For this PR we should aim to have the tests pass with the current CI setup.

You should be able to run this with a fedora toolbox, installing these dependencies https://github.com/tchx84/gtk-apps-testing-docker-action/blob/gjs/Dockerfile and doing this https://github.com/tchx84/gtk-apps-testing-docker-action/blob/gjs/entrypoint.sh

I am trying but no idea how it works. It might be easier to just update the container in use for now

Just to make sure there's no misunderstanding here:

All this change is doing is simply disabling the tests. That's not a solution.

That GH action is not working properly either, reason why I am investigating it on #514 , so let's separate that issue from this PR. This PR is big enough as it is. So,

Please remove this change, or we're are simply going to get stuck here.

I suggest you try:

Creating a Fedora toolbox

Installing the dependencies as per https://github.com/tchx84/gtk-apps-testing-docker-action/blob/gjs/Dockerfile (copy paste)

Running "ninja test"

If there's something that is not working or clear, just ask specific questions.

src/widgets/aboutDialog.js

src/widgets/window.js

Closes tchx84#517

Note that the name of the app is marked as not translatable, this is in line with other uses of it. This will make it so that the default window title is the app name. Closes tchx84#517

This is preferable in GTK 4. Closes tchx84#517

Closes tchx84#517

Its only needed a few times. Closes tchx84#517

This is generally not needed. Closes tchx84#517

Closes tchx84#517

reset-button was used, but it is wrong. Closes tchx84#517

Closes tchx84#517

It's actually used but this can be re-implemented without it. Closes tchx84#517

Closes tchx84#517

The 'destroy' signal is not being emitted. Closes tchx84#517

Closes tchx84#517

Prefer to use default size and the actual property "maximized". Closes tchx84#517

Closes tchx84#517

tchx84 · 2023-04-05T15:09:10Z

@A6GibKm great work 🥳

tchx84 · 2023-04-05T15:16:42Z

@A6GibKm I fixed CI bits and tests are passing now, excellent work!
Let's "freeze" this PR now so I can start doing some cleanups before we merge it.

Ok! Last changes were me trying to remove some unneeded diffs from the earlier commits, but rebasing this PR is not really easy. Besides wiring up a new CI and cleaning the history this should be ready.

EDIT: I see CI passing, did you update the image?

I did :)

A6GibKm force-pushed the gtk4-port branch 2 times, most recently from 23c7118 to 8ba9e6d Compare March 31, 2023 13:33

A6GibKm force-pushed the gtk4-port branch 4 times, most recently from 37972dd to 7522827 Compare March 31, 2023 15:20

A6GibKm force-pushed the gtk4-port branch from 7522827 to 446f435 Compare March 31, 2023 16:01

tchx84 requested changes Mar 31, 2023

View reviewed changes

DaPigGuy reviewed Apr 1, 2023

View reviewed changes

src/widgets/aboutDialog.js Outdated Show resolved Hide resolved

A6GibKm force-pushed the gtk4-port branch 3 times, most recently from 0993373 to f52a15d Compare April 1, 2023 07:46

A6GibKm force-pushed the gtk4-port branch from f52a15d to 6e6e9f3 Compare April 1, 2023 08:10

A6GibKm force-pushed the gtk4-port branch from 39a4bb6 to 051d8e1 Compare April 1, 2023 09:09

A6GibKm force-pushed the gtk4-port branch 5 times, most recently from d8dbb17 to 782ee84 Compare April 1, 2023 12:46

A6GibKm force-pushed the gtk4-port branch from cff1d1c to 1b41285 Compare April 1, 2023 14:01

tchx84 reviewed Apr 1, 2023

View reviewed changes

A6GibKm added 23 commits April 5, 2023 10:59

style: Use newer css color names

41a109d

Closes tchx84#517

pathsViewer: Simplify UI structure

a53f6a8

Closes tchx84#517

window: Make menu button accessible and add back F10 accel

0978706

Closes tchx84#517

window: Make separator not navigatable

da39204

Closes tchx84#517

main: Set application_name to Flatseal

60981b7

Note that the name of the app is marked as not translatable, this is in line with other uses of it. This will make it so that the default window title is the app name. Closes tchx84#517

application: Use control instead of primary

44cd20c

This is preferable in GTK 4. Closes tchx84#517

application: Add accel to close window

7d79060

Closes tchx84#517

undoPopup: Use flat class instead of has-frame

3909fa4

Closes tchx84#517

ui: Remove all unneded uses of hexpand

b248235

Its only needed a few times. Closes tchx84#517

ui: Remove uses of vertical boxes

63207c6

This is generally not needed. Closes tchx84#517

window: Use AdwStatusPage for placeholders

caa0757

Closes tchx84#517

window: Port notifications to AdwToast

d98df2e

Closes tchx84#517

style: Remove unused classes

4dec174

reset-button was used, but it is wrong. Closes tchx84#517

ui: Add popup-completion=False for EntryCompletions

96ea152

Closes tchx84#517

style: Be specific about images with accent color

79b2cb1

Closes tchx84#517

window: Remove applications stack

3353620

It's actually used but this can be re-implemented without it. Closes tchx84#517

window: Simplify app list ui code

1ba4cda

Closes tchx84#517

window: Add padding to sidebar

2073bde

Closes tchx84#517

window: Move shutdown to close_request

82ac24e

The 'destroy' signal is not being emitted. Closes tchx84#517

application: Use GtkUriLauncher for uris

9dad406

Closes tchx84#517

settings: Port saveWindowState to gtk4

be6f70e

Prefer to use default size and the actual property "maximized". Closes tchx84#517

tests: Update to gtk4

a420294

Closes tchx84#517

ui: Remove receives-default

d1b90ea

Closes tchx84#517

tchx84 force-pushed the gtk4-port branch from b397e10 to d1b90ea Compare April 5, 2023 14:59

tchx84 marked this pull request as ready for review April 5, 2023 15:07

tchx84 approved these changes Apr 5, 2023

View reviewed changes

tchx84 merged commit 36af875 into tchx84:master Apr 5, 2023
1 check passed

tchx84 deleted the gtk4-port branch April 5, 2023 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gtk4 port part 2 #515

Gtk4 port part 2 #515

A6GibKm commented Mar 31, 2023 •

edited

tchx84 commented Mar 31, 2023

A6GibKm commented Mar 31, 2023

tchx84 commented Mar 31, 2023

A6GibKm commented Mar 31, 2023 •

edited

tchx84 commented Mar 31, 2023

tchx84 left a comment

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

tchx84 Apr 1, 2023

A6GibKm Apr 2, 2023

tchx84 Apr 3, 2023 •

edited

tchx84 commented Apr 5, 2023

tchx84 commented Apr 5, 2023

Gtk4 port part 2 #515

Gtk4 port part 2 #515

Conversation

A6GibKm commented Mar 31, 2023 • edited

tchx84 commented Mar 31, 2023

A6GibKm commented Mar 31, 2023

tchx84 commented Mar 31, 2023

A6GibKm commented Mar 31, 2023 • edited

tchx84 commented Mar 31, 2023

tchx84 left a comment

Choose a reason for hiding this comment

A6GibKm commented Apr 1, 2023 • edited

A6GibKm commented Apr 1, 2023 • edited

A6GibKm commented Apr 1, 2023 • edited

A6GibKm commented Apr 1, 2023 • edited

tchx84 Apr 1, 2023

Choose a reason for hiding this comment

A6GibKm Apr 2, 2023

Choose a reason for hiding this comment

tchx84 Apr 3, 2023 • edited

Choose a reason for hiding this comment

tchx84 commented Apr 5, 2023

tchx84 commented Apr 5, 2023

A6GibKm commented Mar 31, 2023 •

edited

A6GibKm commented Mar 31, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

A6GibKm commented Apr 1, 2023 •

edited

tchx84 Apr 3, 2023 •

edited