The design of the profile chooser should follow the GNOME HIG, similar to the installer and setup as in bug 7902.
The fancy new profile chooser is now ready for testing. One regression area worth verifying is that buttons show up and work as intended under different profile chooser configurations. For example: - With intro and one active profile - With intro and two active profiles - Without intro and no active profiles - etc. etc. Marking as resolved!
I tested the new profile chooser with 300% scaling in GNOME on a 4k display. In general, things look awesome! The layout is preserved and most things look very sharp and nice. One issue that becomes apparent in HiDPI mode is that the images we add look blurry. The logo should probably be the easiest to fix since that is available as an SVG. The profile images are a mixed bag right now. The new screenshots for GNOME, MATE, and KDE are much larger than the size they appear in the profile chooser. The old screenshots and the icons don't have many extra pixels to use in HiDPI mode, however. At the very least I think we should investigate what can be done here.
We currently don't apply any scaling when actually running the profile chooser with Openbox at the start of a ThinLinc session. I manually ran /opt/thinlinc/libexec/tl-select-profile for testing purposes. Any fixes in this area would likely not be beneficial in reality right now. It would be more future proof though. That the profile chooser isn't HiDPI aware is not reason enough for this bug to be REOPENED.
One issue with the new profile chooser is the icons we use for each profile. The Gnome HIG states that symbolic icons should be used instead: https://developer.gnome.org/hig/guidelines/ui-icons.html
I also noted that we still use the non-symbolic "file missing" icon for profiles without screenshots. This should also be fixed.
The issues mentioned in comment 39, comment 40, and comment 41 have now all been fixed. Marking as RESOLVED.
Unfortunately pycairo's set_device_scale() is too new.. it was introduced in pycairo 1.14.0. My RHEL 7 test machine only has pycairo-1.8.10: > 2022-06-02 16:42:59,760: *** ThinLinc installer started *** > 2022-06-02 16:42:59,761: Architecture: x86_64 > 2022-06-02 16:42:59,762: Distribution: RHEL > 2022-06-02 16:42:59,762: Package format: rpm > 2022-06-02 16:42:59,899: Traceback (most recent call last): > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/tlinstaller", line 743, in <module> > 2022-06-02 16:42:59,899: IIiI11I1I1 ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/tlinstaller", line 695, in IIiI11I1I1 > 2022-06-02 16:42:59,899: II1iii1IiIiii = IiI . run ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/wizard.py", line 110, in run > 2022-06-02 16:42:59,899: return self . _run_gui ( ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/wizard.py", line 141, in _run_gui > 2022-06-02 16:42:59,899: self . _banner_path , self . _banner_scale ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/wizard.py", line 150, in add_intro > 2022-06-02 16:42:59,899: banner_path , banner_scale ) > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/wizard.py", line 92, in _add_intro_or_finish > 2022-06-02 16:42:59,899: IiI11I , Ooo0OO , I1i1Ii , > 2022-06-02 16:42:59,899: File "/home/cendio/server-bundle/libs/modules/thinlinc/tlgtk/factory.py", line 218, in scale_aware_surface_from_pixbuf > 2022-06-02 16:42:59,899: iiOoO0O0OO0 . set_device_scale ( pixbuf . get_width ( ) / width , > 2022-06-02 16:42:59,899: AttributeError: 'cairo.ImageSurface' object has no attribute 'set_device_scale'
Comment #48 has now been resolved. However, there are a number of design/layout changes we need to discuss before considering this bug resolved. Leaving as REOPENED for now.
As mentioned in comment 53 we had a few points up for discussion which I will summarize below: 1. Visibility of profile list descriptions 2. Too small margins around the profile list screenshots 3. The new symbolic icons not behaving as built-in symbolic icons 4. Tab focuses individual profiles in the list instead of just the list itself The first two of these points have already been resolved.
One more point that has come up as part of code reviews: 5. In cases where the profile screenshot aspect ratio isn't 4:3 we want to keep the incorrect ratio but shrink/expand the image to fit the 160x120 space. Currently, the image is skewed to match the wanted 4:3 aspect ratio. Instead, for example, a 320x120 image should be shrunk to 160x60, to fit the 160x120 space while keeping the 8:3 aspect ratio.
To clarify the last point in comment 58, there may be expectations that repeatedly pressing TAB results in the following focus order: 1. Profile list (and not individual list items) 2. Back/Quit button 3. Start button 4. Profile list 5. ... These expectations come from an analogy with radio buttons where TAB focuses the radio group and not the individual buttons. The individual buttons can then be focused using the arrow keys when their radio group is in focus. Note that the above is not the case in all GUI toolkits: - GTK2 has this behavior - Older windows applications have this behavior (for example the legacy control panel) - Newer windows applications don't have this behavior (for example the Settings application which replaces the legacy control panel) - Parts of macOS have this behavior while other parts don't (see for example System Preferences which has a mix of both) - FLTK doesn't have this behavior
(In reply to William Sjöblom from comment #58) > 1. Visibility of profile list descriptions Fixed! > 2. Too small margins around the profile list screenshots Fixed! > 3. The new symbolic icons not behaving as built-in symbolic icons This is not as trivial as first thought with user defined icons, so we'll postpone this and either implement symbolic icons properly or remove the built-in profile icons entierly. > 4. Tab focuses individual profiles in the list instead of just the list > itself Fixed using a workaround for https://gitlab.gnome.org/GNOME/gtk/-/issues/4977 (In reply to Samuel Mannehed from comment #59) > 5. In cases where the profile screenshot aspect ratio isn't 4:3 we want to > keep the incorrect ratio but shrink/expand the image to fit the 160x120 > space. Currently, the image is skewed to match the wanted 4:3 aspect ratio. > Instead, for example, a 320x120 image should be shrunk to 160x60, to fit the > 160x120 space while keeping the 8:3 aspect ratio. This is the last remaining thing to be fixed before resolving.
The remaining point mentioned in comment 63 has now been fixed. Marking as resolved.
The documentation for the profile settings looks like it still needs to be adjusted for the new layout.
It also seems like we no longer can set a greeting containing Pango markup. According to the documentation, this should be possible.
One more thing that we need to take a stance to is the fact that we no longer display the profile chooser Greeting on the profile list page. This can be considered a breaking change since the greeting may be used to communicate vital information to the user on systems where the introduction text is disabled. Just adding the greeting to the top of the profile list is one possibility, but since the list is now scrollable (and will automatically scroll down to the default profile) we still run the risk of the greeting not being visible. Additionally, I think it would be hard to do this without sacrificing the general neatness of the new profile list. Another approach is to just accept the fact that we just display the greeting on the intro page. This would require the following (possibly non-exhaustive) list of changes: - Updating the layout of the tlwebadm intro module such that the checkbox appears before the greeting configuration field. - Updating the documentation (tlwebadm_profiles.rst, config_profiles.rst) to reflect this new behavior. - Possibly renaming the configuration variable and associated UI/documentation text to something more bound to the intro page. - Writing a release not detailing the change and tagging this bug with relnotes.
* The documentation has now been updated to reflect the new layout (as highlighted in comment 66) * A small heading label for displaying the greeting message has been added to the profile list page (as highlighted in comment 68) * The greeting labels now correctly render Pango markup (as highlighted in comment 67) Marking as resolved.
Created attachment 1040 [details] screenshot of the error page in tl-select-profile The error page of the profile chooser looks cramped on Ubuntu 22.04, we should increase the margins of that page.
(In reply to Samuel Mannehed from comment #75) > The error page of the profile chooser looks cramped on Ubuntu 22.04, we > should increase the margins of that page. This is due to Ubuntu setting the system-wide GTK (icon) theme to Yaru instead of the default Adwaita. For some reason, Yaru does not include padding in its icons which Adwaita does. I've had a look at SLES15, Fedora 35, and RHEL8 (which all stick to the default Adwaita theme), and things look good there. I don't see how this warrants a workaround since it's only an issue on Ubuntu. Furthermore, the error page should hopefully not be shown especially often. Nonetheless, the error page layout could benefit from more air overall, so I'll go ahead and turn up the margins a bit more. Hopefully, this will relieve the Ubuntu issue a bit.
The issue described in comment 75 has now been slightly relieved. See comment 76.
The profile chooser crashes if I specify a screenshot that doesn't exist: > Traceback (most recent call last): > File "/opt/thinlinc/libexec/tl-select-profile", line 118, in <module> > iI111iiIi11i ( ) > File "/opt/thinlinc/libexec/tl-select-profile", line 108, in iI111iiIi11i > OOO00oO0oOo0O = IiII . run ( ) > File "/opt/thinlinc/modules/thinlinc/profiles/chooser.py", line 66, in run > self . _add_entry ( IiII ) > File "/opt/thinlinc/modules/thinlinc/profiles/chooser.py", line 295, in _add_entry > Iii1I1I1 = tlgtk . GdkPixbuf . Pixbuf . new_from_file ( IIiIIIII1iiI ) > gi.repository.GLib.GError: g-file-error-quark: Failed to open file “/opt/thinlinc/share/tl-select-profile/terminal.png”: No such file or directory (4) This is a regression, the old profile chooser didn't crash in this scenario. It fell back on a error-icon instead of a screenshot.
Specifying screenshots that doesn't exists or isn't images now give image-missing icons instead of crashing.
I have verified the following using build 2706 on Ubuntu 22.04 and RHEL 7: ✓ The profile list doesn't show when only one profile is available ✓ The intro page is shown if "show_intro = true" ✓ The only available starts automatically ✓ The profile list looks good when listing 6 different profiles ✓ The error page is shown when no profiles are available ✓ The margins of the error page now look good ✓ Starting a profile by double-clicking on it ✓ The "Back", "Quit" and "Start" buttons work as expected when clicked ✓ Keyboard navigation works well ✓ Starting the selected profile by pressing ENTER ✓ Navigating to a different profile works ✓ Navigating to "Quit" and then aborting works ✓ If "default" is set the profile chooser will automatically scroll to that profile ✓ Changing "order" works ✓ Non existent profiles in "order" doesn't show up ✓ Changing "introduction" works ✓ Setting "show_intro" to false works ✓ Changing "greeting" works for both the intro page and the profile page ✓ When "greeting" is empty it doesn't take up space above the profile list ✓ The new icons look nice ✓ Icon specified with only a name like "terminal" or "addressbook" works ✓ Empty icon works (no icon is shown) ✓ An icon with an incorrect aspect ratio works (it's stretched) ✓ An icon that is too small works ✓ An icon that is too big works ✓ The new screenshots look nice ✓ Empty screenshot works (image-missing icon is shown) ✓ A screenshot with an incorrect aspect ratio works (the ratio is preserved) ✓ A screenshot that is too small works ✓ A screenshot that is too big works ✓ Migrating custom profile configuration when upgrading from 4.14.0 works well Closing.
I found a bunch of places where the spacing between elements isn't the recommended [1, 2] multiple of 6. Reopening… • [1] <https://developer-old.gnome.org/hig/stable/visual-layout.html.en#general-guidelines> • [2] <https://blogs.gnome.org/tbernard/tag/design/#headerbar>
The spacing issues highlighted in comment 85 have now been fixed. Marking as resolved.
Rechecked all three pages of the profile chooser on Ubuntu 22.04 and SLES 12. The layout looks good to me.
With only one profile available, the profile chooser intro page claims the "OK" button will start the session while it's actually labeled "Start". Seen on build #2728.
The default intro text has now been changed to reflect the new button labels. Marking as resolved.
Looks correct now. I was not able to find any other incorrect references in documentation or UI text, in either tl-select-profile, tl-setup or tlinstaller.
