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

UI Improvements #252

Closed
wants to merge 9 commits into from
Closed

UI Improvements #252

wants to merge 9 commits into from

Conversation

Wartybix
Copy link

@Wartybix Wartybix commented Nov 7, 2024

Hello,

Firstly, thank you for creating this extension. It is one of my favourites, and it's nice to see a lovely new background every day on my desktop.

I've made some changes to the UI, mainly affecting the preferences window. In each of my commits, I've made a particular change, and have justified why in the commit message details. Overall, the changes are to make the UI more in line with GNOME's Human Interface Guidelines, making it easier to use. Notable changes are:

  • Changed text to match GNOME HIG writing style. Namely, applying header capitalization to headers and labels and such -- for example, changing the 'Shuffle wallpaper' label to 'Shuffle Wallpaper'. I've done this by adding a new English (en) translation file with these modifications, rather than changing the strings in the source code, so it doesn't break and 'un-translate' other translations.
    • Note: I removed the Australian (en_AU) translation, as after looking at the PO file, I didn't see any meaningful changes it made to adapting text to locale, such as different spellings etc.. It was also years out of date, and would cause conflicts with the new English (en) translation file for no reason.
  • Changed some of the page header icons, as I believe these make more sense than before. For example, changing the 'Settings' icon to applications-system-symbolic, the 'Lock Screen' icon to system-lock-screen-symbolic, etc..
  • Removed icons from buttons with labels, as the GNOME HIG on buttons state that "outside of header bars, buttons should contain either an icon or a label, and not both".
  • Added labels to the link buttons in the 'About' page, rather than keeping them as invisible 'ghost' buttons. This allows the user to hover over them and preview the URL before clicking. It also removes that quirkiness of a random rectangle glowing when the action row is clicked on, and the link button is activated -- the button still glows when the row (rather than the button) is clicked on, but now it makes more sense with a label.
  • The version number is now represented by a subtitle in a property row, rather than a button that doesn't do anything.
  • The app icon now shows in the About page (before, there was just a large, empty GtkImage where I assume the app icon was supposed to go).
  • Improved Gallery page UI. Removed redundant shuffle controls, as these took up a lot of space, were already on the Settings page, and didn't fit with the purpose of the rest of the Gallery page IMO. I also made the button column for each image linked, so it looks nicer and more 'grouped'-looking.

Attached are screenshots of the preferences window with the improvements:
Screenshot From 2024-11-07 08-18-40
Screenshot From 2024-11-07 08-18-45
Screenshot From 2024-11-07 08-18-51
Screenshot From 2024-11-07 08-18-55
Screenshot From 2024-11-07 08-19-04

This translation modifies UI text to follow GNOME HIG writing style
guidelines
(https://developer.gnome.org/hig/guidelines/writing-style.html) in
English, without breaking other translations. Modifications include
header capitalization, and ellipses on the 'Change Folder' button to
indicate further action required.
Reason: The translation made no meaningful changes to the message IDs
(in terms of adapting it to locale), and was out of date anyway. It
would just conflict with the new 'en' translation for no real reason.
Reason: I think these icons better reflect the page contents.
Reason: Outside of header bars, buttons should contain either an icon or
a label, and not both
(https://developer.gnome.org/hig/patterns/controls/buttons.html).
Note: while not having labels does have a cleaner look, having labels on
the buttons allows the user to hover over them and preview the URLs
before they click on them. Also, prevents that 'ghost button' highlight
when the action row is clicked and the button is activated -- it makes
more sense for it to highlight with a label.
Reason: version_button was not a button, so its purpose is clearer as
just text in a property row.
Reason: It makes more sense to have to have the shuffle mode option in
only one place (the Settings page), than to have it appear twice. Plus,
it allows the Gallery page to shine as a place to view and manage the
user's individual Bing backgrounds, without the shuffle controls kind of
randomly mishmashed with it.
The buttons look nice this way.
@neffo
Copy link
Owner

neffo commented Nov 8, 2024

Wow this is amazing, thanks for your effort. I'm happy to merge this into the next version (51).

Is it possible you can base it off this branch? https://github.com/neffo/bing-wallpaper-gnome-extension/tree/version-51

I've done this by adding a new English (en) translation file with these modifications, rather than changing the strings in the source code, so it doesn't break and 'un-translate' other translations.

I think we could just bulk find & replace this in all the translations and in the source if you'd like. That is how I've made these changes in the past, although maybe not to the same degree. We rebuild all the compiled translations at build anyway.

This is the only comment I have on these changes other than I agree. I've not across the HIGs but of course would prefer the extension follows it.

Did you have further changes in mind with this PR?

@Wartybix Wartybix mentioned this pull request Nov 8, 2024
@Wartybix
Copy link
Author

Wartybix commented Nov 8, 2024

Okay, I have submitted a new pull request #253, based off of the version-51 branch 😁. There are some conflicts though, so do you want me to make some changes on my end, or will you resolve them? I suppose it makes more sense to talk about that in the new PR's comment thread though.

I think we could just bulk find & replace this in all the translations and in the source if you'd like.

That sounds good -- it's up to you though. I just made the text style changes as a new 'en' translation to try to keep the amount of code review and extra work for this small (arguably nit-picky) change as low as possible. Functionally it should work the same, using either the new 'en' translation, or using your method of find-and-replacing everything. Your idea does have the benefit of making it easier for future translators, as changing the message IDs to use heading styles etc. tells the translators that translations should also have those same styles.

I don't think I have anything else to add to the PR unless you want me to make some modifications. I was actually using this extension as a reference while making my first GNOME extension (called Auto Accent Colour, which I've recently submitted to the extension store for review and works quite nicely alongside yours, as it automatically sets your GNOME 47+ accent colour from colours in your wallpaper). Anyway, once I uploaded my extension, I just thought it would be fun to make some HIG-conforming improvements to yours after spending time looking at your source code and preferences window.

@Wartybix Wartybix closed this Nov 8, 2024
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.

2 participants