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

Wrong icon of "View last log" in backintime-qt #1386

Closed
hannes101 opened this issue Dec 8, 2022 · 6 comments
Closed

Wrong icon of "View last log" in backintime-qt #1386

hannes101 opened this issue Dec 8, 2022 · 6 comments
Assignees
Labels
Cosmetics appearance, icons, themes

Comments

@hannes101
Copy link

hannes101 commented Dec 8, 2022

I am using backintime-qt on Xfce, so under a GTK+ environment. I am always a bit confused, because the "View last log" entry in the menu bar is associated with a "Create new file" icon.
image
It doesn't really change with the icon theme, it's just that when using some icon themes the icons are not shown, but only text.
I am not sure, which icon might be better suited, but I always find the current one confusing.
image
If there's a rationale behind this, then I am fine with it.

@buhtz
Copy link
Member

buhtz commented Dec 8, 2022

Dear Hannes,
thank you very much for your report.

I agree that this icon doesn't make much sense. I assume the original "rational" was to use an Icon different from the one on it's left side ("last snapshot protocol").

Because there is a lot more work to do on the GUI I would suggest keeping a fix as simple as possible; quick and easy without wasting to much resources. The "real" re-design of that GUI needs to be done in the future.

My approach would be

  • remove "view last log" icon
  • remove "view last snapshot protocol/log" icon
  • add a new menu entry "Logs" (or Protocols?) in the menubar
  • add two menu items into that menu for the two removed icons
  • also remove in the "view" menu the two duplicate entries for "view last log" and "view last snapshot log".

Or much more simpel: Just do remove the two menu icons and keep their corresponding menu entries in the "View" menu.

My thought behind removing icons from a menubar is that icons there are intended to be used very often. The icon is there for quick access. But viewing log files is not a task done very often. I see on need for such icons a toolbar.

EDIT:
All of the possible solutions are easy. I sill identified the relevant parts in the code. I vote for removing the icons and adding an extra "Logs" menu in the main menu.

EDIT2:
Maybe you (@hannes101 ) would like to contribute code here? It is a good starting issue to become warm with the project and its repo. We will assist you if you need.

@aryoda
Copy link
Contributor

aryoda commented Dec 9, 2022

I agree that our icons (like above one) are sometimes confusing or non-standard.

After our stabilization release one of our bigger tasks may be to improve the usability of the GUI (incl. code refactoring to enable unit testing of GUI-related code) and I would prefer to wait with this change until we tackle the GUI refactoring.

This would also minimize the "confusion" of users ("where is my button").

Anyhow changing the icon is trivial, since it is assigned to the button here

self.btnLastLogView = self.mainToolbar.addAction(icon.VIEW_LAST_LOG, _('View Last Log'))

and loaded here:

VIEW_LAST_LOG = QIcon.fromTheme('document-new')

[...] in some icon themes the icons are not shown, but only text.

That is correct and is caused by three possible reasons:

  1. Non-standard icon theme: The icon scheme does not implement all standard icons defined by the freedesktop.org icon spec

  2. Icons not installed: Your active scheme does support the freedesktop.org icon spec but the icons are not installed.
    Quite often there are two packages for a scheme: The visuals/layout and the icons and both packages
    need to be installed to fully provide the icons.

  3. BiT switches to another icon scheme: If the current one does not provide the BiT "Logo" icon ("document save") BiT probes three different themes. If none of them is installed the oxygen theme is used (even if it is not installed and the icons are therefore missing):

    backintime/qt/icon.py

    Lines 23 to 26 in 55580a1

    for theme in ('ubuntu-mono-dark', 'gnome', 'oxygen'):
    if not QIcon.fromTheme('document-save').isNull():
    break
    QIcon.setThemeName(theme)

I am preparing a fix for the "missing icon" issues currently (follow the master issue for that: #1306).
I still have to understand some missing icon issues when running as root before I send a PR with my fix.

@buhtz
Copy link
Member

buhtz commented Dec 9, 2022

I agree to (minimally) wait until our next release before doing bigger things on the GUI. But refactoring the GUI will IMHO take years and is not just a one-release-step.

VIEW_LAST_LOG = QIcon.fromTheme('document-new')

[...] in some icon themes the icons are not shown, but only text.

That is correct and is caused by three possible reasons:

But the key of that issue here is not about a missing but a wrong icon. The loaded icon here is document-new which is incorrect because there is no new file created but an existing one is opened.

I still vote for removing that two log icons. I assume that this toolbar buttons are not used very often. And the functionality itself remains in the "view" menu. The change is minimal and trivial.

Waiting until GUI refactoring is IMHO to long and will take multiple releases.

@aryoda aryoda added the Cosmetics appearance, icons, themes label Dec 9, 2022
@aryoda
Copy link
Contributor

aryoda commented Jan 23, 2023

We could use the standard freedesktop.org icon "document-open-recent" for the "view last log" toolbar button.

It looks like this in the toolbar (using xcb with the qt5ct platform theme and the fusion style):

Screenshot from 2023-01-23 02-01-20

I am not really happy with this icon but it is the standard and IMHO we should not invent other semantics for existing icons.

The current icon with the "plus" symbol is:

Screenshot from 2023-01-23 02-06-42

Edit: We could even also use the standard "document-open" icon for "view snapshot log"
(instead of the currently used "text-plain" mime type icon):

Screenshot from 2023-01-23 02-26-48

Edit:

I still vote for removing that two log icons. I assume that this toolbar buttons are not used very often. And the functionality itself remains in the "view" menu. The change is minimal and trivial.

Removing the two "view ... log" buttons from the toolbar does not solve the decision about the best icon since the icon is still shown in the menu.

@hannes101
Copy link
Author

I think the "document-open-recent" makes sense in that context. However, I would rather not change the other log icon to "document-open", which in my opinion doesn't fit nicely to that particular use.

@buhtz buhtz added this to the 1.3.5 or 1.4.0 milestone Mar 7, 2023
@aryoda
Copy link
Contributor

aryoda commented Jul 17, 2023

I have fixed all known systray and app icon issues (hopefully) with my PR #1480 (on dev branch)
and this issue is contained too:

VIEW_LAST_LOG = QIcon.fromTheme('document-open-recent') # 'document-open-recent') # ('document-new')

Re-testing and feed-back welcome (but not critical for this issue).

@aryoda aryoda self-assigned this Jul 17, 2023
aryoda added a commit that referenced this issue Sep 7, 2023
I have forgot to mention this fix (contained in PR #1480) in the CHANGES
@aryoda aryoda closed this as completed Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cosmetics appearance, icons, themes
Projects
None yet
Development

No branches or pull requests

3 participants