Skip to content

Preferences/About: draw links with a more readable color, improve defs_urls usage#3464

Merged
Holzhaus merged 11 commits into
mixxxdj:2.3from
ronso0:pref-link-color
Jan 6, 2021
Merged

Preferences/About: draw links with a more readable color, improve defs_urls usage#3464
Holzhaus merged 11 commits into
mixxxdj:2.3from
ronso0:pref-link-color

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented Dec 17, 2020

This aims to guarantee readbility of hyperlinks and file links in Preferences and About.
I added some helpers to blend the palette's text color and the link text color. This should give readable links no matter if the user's theme has the link color defined or not.

In the end the color fix was just a very quick fix but I stumbled upon some inconveniences with how links are created.

  • added blendColor() to util/color/color.h to belnd two colors 50/50
  • added a small utility to util/string.h to simplify link formatting in ui files:
    new class LinkFormatter, QString coloredLinkString
  • added presetSupportLinks() to concentrate creation of controller mapping support links
  • some tweaks along the way

Note: this moves some tr strings from ui files to cpp, so we may again need to push the tr sources to transifex.

Previously, I could hardly see the controller file links with my (official) dark OS theme. It seems there's no adequate color defined for links and some Qt default is picked (blue):
image

Now, they're drawn with a blend of the palette's text color / link color:
image

@github-actions github-actions Bot added the ui label Dec 17, 2020
@ronso0 ronso0 added this to the 2.3.0 milestone Dec 17, 2020
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 17, 2020

Sure the link color issue is no bug on our side but (considering myself an advanced user) I couldn't fix it with OS appearance settings or Qt5 Settings so I guess this hack is okay.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 17, 2020

WIP again. while trying to understand the clazy warnings I decided to move that link formatting utility to new src/util/linkformatter

@ronso0 ronso0 marked this pull request as draft December 17, 2020 19:48
@ronso0 ronso0 force-pushed the pref-link-color branch 2 times, most recently from b61789c to fa5dd5f Compare December 17, 2020 23:09
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 17, 2020

Okay, done.
The link string builder utility is now a new class in util/string.h Okay @uklotzde ?

@ronso0 ronso0 changed the title Preferences/About: draw links with a more readable color Preferences/About: draw links with a more readable color, improve defs_urls usage Dec 18, 2020
Comment thread src/util/string.h Outdated
@ronso0 ronso0 marked this pull request as ready for review December 20, 2020 19:44
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 27, 2020

@Holzhaus Can you check how that looks like with i3wm?

@ronso0 ronso0 requested a review from daschuer December 27, 2020 16:47
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Jan 3, 2021

reminder for myself: check QString > QStringLiteral
Not here, not now...

Comment thread src/util/string.h Outdated
Copy link
Copy Markdown
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@Holzhaus
Copy link
Copy Markdown
Member

Holzhaus commented Jan 6, 2021

Waiting for CI.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Jan 6, 2021

Waiting for CI.

all greeeen :)

@Holzhaus Holzhaus merged commit d688a25 into mixxxdj:2.3 Jan 6, 2021
@ronso0 ronso0 deleted the pref-link-color branch January 6, 2021 00:50
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Jan 6, 2021

I'm not sure how I made a0e7911 slip in here... whoopsy!
gave me some headaches when I merged 2.3 into main locally

@uklotzde
Copy link
Copy Markdown
Contributor

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Jan 10, 2021

yup, from a first glance it looks like.
but I built before we merged, also CI passed, and res builder would fail if the icons are not found.
also #3535 CI succeeded..

@uklotzde Please give the final Okay for #3484 so i can squash and merge to 2.3, then merge 2.3 into main.

ronso0 added a commit that referenced this pull request Jan 10, 2021
fix the regression from #3464 and merging 2.3 to main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants