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

Change the Nations button blink to a gentle notification emblem #1698

Merged
merged 3 commits into from
Jan 13, 2023

Conversation

lmoureaux
Copy link
Contributor

@lmoureaux lmoureaux commented Jan 5, 2023

This minimizes the annoyance while still hinting that something in the tab is active. The notification icon uses the same design as in the Messages button.

Closes #1676.
Backport candidate.

Also cleaned up the SVG to be easier to edit.
This minimizes the annoyance while still hinting that something in the tab is
active.

Closes longturn#1676.
@lmoureaux lmoureaux requested a review from jwrober January 5, 2023 01:08
@jwrober
Copy link
Collaborator

jwrober commented Jan 6, 2023

The overall change is more appealing and does match the messages icon. however i think the change is missing a refresh. after canceling two meetings... i would expect to see base flag.
image

@lmoureaux
Copy link
Contributor Author

Opened two meetings, restarted the client, closed them:
image
The icon you see is strange, any idea where it comes from?

@jwrober
Copy link
Collaborator

jwrober commented Jan 7, 2023

It looks like an artifact of the old image with the dot on it. Is there a way to clear the old icon as part of the refresh?

@jwrober
Copy link
Collaborator

jwrober commented Jan 7, 2023

Another point of note, I left one savegame with the icon "bad" and then opened another savegame and the icon did not revert to "no meetings" mode, it was still in a bad state.

@lmoureaux
Copy link
Contributor Author

Another point of note, I left one savegame with the icon "bad" and then opened another savegame and the icon did not revert to "no meetings" mode, it was still in a bad state.

Would you have the save?

@jwrober
Copy link
Collaborator

jwrober commented Jan 7, 2023

This is the first save that I have that starts with two meetings at load.
freeciv21-T0325-Y01877-auto-LateTestGame.sav.gz

This is the second one I did in my last comment that started with the "artifact"
freeciv21-T0048-Y-1650-manual.sav.gz

@jwrober
Copy link
Collaborator

jwrober commented Jan 7, 2023

Ok something is up with my local. I was looking at icon files for the PR on allies button and noticed this
image

So somehow my flag.svg got messed up.

@jwrober
Copy link
Collaborator

jwrober commented Jan 7, 2023

The resize of flag.svg get's corrupted on my local when I bring down the patch for testing. I wiped my local to get it "clean" and when I run the eval screen my local copy of flag.svg is corrupted to the image above. I think the resize needs some work I guess, or just leave it as it was as it worked fine before.

Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

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

See comment above

@lmoureaux
Copy link
Contributor Author

Can't see anything in the svg that would cause this issue. Is the file partially merged?

$ md5sum data/themes/gui-qt/icons/flag.svg
9d1e89c654f16e03c59ffa4829a17237  data/themes/gui-qt/icons/flag.svg

Qt 5.15.3 has problems rendering the new version.

See longturn#1698.
See QTBUG-92184.
@lmoureaux
Copy link
Contributor Author

Changes to flag.svg reverted.

@lmoureaux lmoureaux requested a review from jwrober January 7, 2023 23:55
@lmoureaux
Copy link
Contributor Author

(For the record, this is a QtSvg bug)

Copy link
Collaborator

@jwrober jwrober left a comment

Choose a reason for hiding this comment

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

How do I find all the upstream bugs?! Reverted flag.svg works great.

Backport candidate recommended.

@lmoureaux
Copy link
Contributor Author

How do I find all the upstream bugs?!

I looked at the commit log between your QtSvg and mine (which has the bug fixed). There were like 3 changes, and only one commit message looked relevant.

@jwrober
Copy link
Collaborator

jwrober commented Jan 13, 2023

I'm going to document the change in another PR, so merging this now.

@jwrober jwrober merged commit 5c5e5ee into longturn:master Jan 13, 2023
@lmoureaux lmoureaux deleted the bugfix/blink branch January 30, 2023 00:41
@lmoureaux lmoureaux self-assigned this Jan 30, 2023
lmoureaux added a commit that referenced this pull request Feb 2, 2023
Qt 5.15.3 has problems rendering the new version.

See #1698.
See QTBUG-92184.
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.

Create an option to enable/disable the binking Nations and Research view buttons
2 participants