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

FiltersDialog now uses a color picker for color selection #194

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

safield
Copy link

@safield safield commented Dec 24, 2017

-FiltersDialog no longer uses a drop down box of predefined SVG color strings. In place it uses two push buttons that launch a QColorDialog. This allows for custom color specification.

-refactored Filter class to use QColor for its fore and back colors, instead of the old approach which uses strings. This reduces coupling with UI.

-incrememnted FilterSet::FILTERSET_VERSION to 2, since Filters now store custom colors. These colors are saved in config as hex strings. BACKWARDS COMPATIBILITY IS MAINTAINED WITH VERSION 1 CONFIG FILES. (made the assumption that backwards compatibility should always be maintained.)

-FiltersDialog no long uses a drop down box of predefined SVG color strings. In place it uses two push buttons that launch a QColorDialog. This allows custom color specification.

-refactored Filter class to QColor for its fore and back colors, instead of the old approach which uses strings. 

-incrememnted FilterSet::FILTERSET_VERSION to 2, since Filters noW store custom colors. These colors are saved in config as hex strings. BACKWARDS COMPATIBILITY IS MAINTAINED WITH VERSION 1 CONFIG FILES.
@safield
Copy link
Author

safield commented Dec 25, 2017

I believe the travis-ci clang build is broken, unrelated to this commit.

@variar
Copy link
Contributor

variar commented Dec 25, 2017

Qt in brew has been updated to 5.10. Seems like this version does not support macos 10.7. Changing to 10.8 in glogg.pro should fix ci build.

@safield
Copy link
Author

safield commented Dec 25, 2017

Should that be fixed in the mainline, then I pull in the commit to my fork, then resubmit pull request?

Are pull requests ever accepted with failed continuous integration builds?

@variar
Copy link
Contributor

variar commented Dec 25, 2017

I've created PR #202 to address ci issue

@safield
Copy link
Author

safield commented Nov 22, 2018

Has this pull request not been accepted because the design direction is not desirable, or because the code implementation is not of a high enough quality?

Or is it just a matter of free time to review it?

I would be willing to address the issues, if there were some.

variar added a commit to variar/klogg that referenced this pull request Jan 30, 2019
danberindei pushed a commit to danberindei/glogg that referenced this pull request Apr 21, 2021
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