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

Inherit text color for filter list elements #11998

Merged
merged 1 commit into from Feb 14, 2020
Merged

Conversation

@Kolcha
Copy link
Contributor

Kolcha commented Feb 11, 2020

Filter list (left side panel) in WebUI is implemented using tags,
CSS defines default style for all elements, and specific style for
filter list elements. Default style for elements sets color, and
this color also used in list. This is looks not so well. So lets just
inherit text color from parent element, and as so as it is not set, so
default text color will be used.
This makes filter list looks like other UI elemets, making all UI more
consistent (like in desktop app).

P.S> I'm not web developer, I know too little in web-related stuff. This change was experimentally achieved using Firefox' web developer tools.

before:
Screenshot 2020-02-11 22 18 53
after:
Screenshot 2020-02-11 22 27 37
Note color change: orange -> dark gray. As for me, it looks much better, especially if a lot of torrents in the list.

@thalieht thalieht added the WebUI label Feb 11, 2020
@Chocobo1 Chocobo1 added this to the 4.2.2 milestone Feb 12, 2020
@Chocobo1 Chocobo1 requested a review from Piccirello Feb 12, 2020
@Chocobo1

This comment has been minimized.

Copy link
Member

Chocobo1 commented Feb 12, 2020

Seems your commit message title is not correct (or precise enough). You're now inheriting the color from the parent element instead of using the default color. When the color property is not specified, it is using the default color.

@Piccirello

This comment has been minimized.

Copy link
Member

Piccirello commented Feb 12, 2020

I like this change. It makes sense to style these sidebar items like our other UI controls, rather than like a link.

Filter list (left side panel) in WebUI is implemented using <a> tags,
CSS defines default style for all <a> elements, and specific style for
filter list elements. Default style for <a> elements sets color, and
this color also used in list. This is looks not so well. So lets just
inherit text color from parent element, and as so as it is not set, so
default text color will be used.
This makes filter list looks like other UI elemets, making all UI more
consistent (like in desktop app).
@Kolcha Kolcha force-pushed the Kolcha:filter-list branch from 9932007 to 1e59dcd Feb 12, 2020
@Kolcha Kolcha changed the title Use default text color for filter list elements Inherit text color for filter list elements Feb 12, 2020
@Kolcha

This comment has been minimized.

Copy link
Contributor Author

Kolcha commented Feb 12, 2020

commit message updated according to notes

@NotTsunami

This comment has been minimized.

Copy link
Contributor

NotTsunami commented Feb 12, 2020

Shouldn't the commit title still be prefixed with WebUI: ?

@Chocobo1

This comment has been minimized.

Copy link
Member

Chocobo1 commented Feb 14, 2020

Shouldn't the commit title still be prefixed with WebUI: ?

That hasn't become a convention yet. So as long the PR adheres to CODING_GUIDELINES.md, it will be fine.

@Chocobo1 Chocobo1 merged commit d5a4ebe into qbittorrent:master Feb 14, 2020
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Chocobo1

This comment has been minimized.

Copy link
Member

Chocobo1 commented Feb 14, 2020

@Kolcha
Thank you!

@Kolcha Kolcha deleted the Kolcha:filter-list branch Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.