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

Remove libgnomekbd #5417

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Remove libgnomekbd #5417

merged 1 commit into from
Jan 24, 2024

Conversation

jexposit
Copy link
Contributor

libgnomekbd is a C library used by Anaconda to display the keyboard preview widget. It is stuck in GTK 3 and X11 (libxklavier).

Replace it with Tecla.

@pep8speaks
Copy link

pep8speaks commented Jan 22, 2024

Hello @jexposit! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-01-22 16:36:25 UTC

@jexposit
Copy link
Contributor Author

/build-image --boot.iso

libgnomekbd is a C library used by Anaconda to display the keyboard
preview widget. It is stuck in GTK 3 and X11 (libxklavier).

Replace it with Tecla.
Copy link

Images built based on commit dfaf84d:

  • boot.iso: success

Download the images from the bottom of the job status page.

Copy link
Contributor

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks a lot! :)

I also really like how the tecla process is cleaned up when you click outside outside the window & IMHO perfectly appropriate here where one does not enter any data, unlike with the connection editor with its --keep-above, where it makes sense to not loose the window. :)

Copy link
Member

@jkonecny12 jkonecny12 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

@jkonecny12 jkonecny12 added release note required Write a release note for this change. and removed documentation labels Jan 23, 2024
@rvykydal
Copy link
Contributor

rvykydal commented Jan 23, 2024

Looks good to me, but when I tried the iso generated in the comment above, maximizing the tecla window with icon and then clicking minimize icon gets me stuck in it.

Screenshot from 2024-01-23 10-49-37

@jkonecny12
Copy link
Member

BTW, working on resolving the failed test. I'm getting you into the Cockpit CI allowed users.

@jexposit
Copy link
Contributor Author

Nice catch @rvykydal. It looks like a GNOME Kiosk bug. I reported it upstream:
https://gitlab.gnome.org/GNOME/gnome-kiosk/-/issues/20

Meanwhile, I'll investigate if it is possible to workaround this issue by disabling the maximize and minimize buttons, as they are not useful for us.

nm-connection-editor also displays them, but clicking them has no effect.

@jexposit
Copy link
Contributor Author

jexposit commented Jan 23, 2024

Adding a file in /etc/gtk-4.0/settings.ini with content:

[Settings]
gtk-decoration-layout=:close

Removes the minimize and maximize buttons.

However, it looks like this used to work:
https://github.com/rhinstaller/anaconda/blob/master/data/window-manager/config/org.gnome.desktop.wm.preferences.gschema.override#L2

But at some point it got broken. It looks like a different bug to fix in our side.

@rvykydal
Copy link
Contributor

To be clear, personally I wouldn't block the PR on the min/max buttons issue, thank you for investigating it. We could handle it as a separate issue, but I'll leave the decision on you.

@M4rtinK M4rtinK merged commit 99a581a into rhinstaller:master Jan 24, 2024
16 of 17 checks passed
@jexposit
Copy link
Contributor Author

Here is the fix for the minimize and maximize buttons:
#5421

Marked as draft as I had a few questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f40 release note required Write a release note for this change.
Development

Successfully merging this pull request may close these issues.

5 participants