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.

Sign up for GitHub

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

Jump to bottom

Fix QPushButton with color, used for color picking feedback. #672

Merged
merged 3 commits into from
Oct 10, 2017
Merged

Fix QPushButton with color, used for color picking feedback. #672

merged 3 commits into from
Oct 10, 2017

Conversation

guitorri
Copy link
Member

Close #74.

Based on PR #662.

@guitorri
Copy link
Member Author

Oops something wrong here... need to rebase.

@guitorri guitorri mentioned this pull request Feb 18, 2017
Closed
@guitorri
Copy link
Member Author

No no, it indeed depends on #662.

@guitorri guitorri force-pushed the push_button_color branch from 7f858db to 4d2fafd Compare July 12, 2017 18:32
@guitorri
Copy link
Member Author

Before:
screen shot 2017-07-12 at 20 39 33

After:
screen shot 2017-07-12 at 20 35 51

@guitorri guitorri added this to the 0.0.20 milestone Jul 12, 2017
@in3otd
Copy link
Contributor

in3otd commented Jul 15, 2017

...well the stylesheet solution looked nicer here... when it worked

Several "color picking" buttons have a text string consisting of a few spaces (" "), this causes the color pixmap there to be out of center, especially when the dialog is resized here. Removing the spaces the behavior seems ok.

Could a similar solution also work for all the color-picking buttons in qucssettingsdialog.cpp ?

@guitorri
Refactor setter getter for colored buttons.
9b6f495 
Due to platform differences creating a simple color picker button seems
difficult.
See discussion in issue Qucs#74.

This patch used part of PR Qucs#135 to implement the button with a pixmap on
it. Stylesheets could be used instead.

This patch also pulls all the get/set of colors into buttons into misc::
@guitorri
Copy link
Member Author

Cleaned up the white spaces and pulled all the color setter/getters into misc::
The issue with stylesheets in my system is that the button only accepts clicks on a portion of the area.

@guitorri
Copy link
Member Author

guitorri commented Oct 9, 2017

Can someone check on Linux o see if this makes sense?

@andresmmera
Copy link
Contributor

I can check that tomorrow...

@andresmmera
Copy link
Contributor

I've just tested this PR in Ubuntu and I didn't find issues.

However, I've found the following warning:

graphictextdialog.cpp:34:67: warning: unused parameter ‘name’ [-Wunused-parameter]
GraphicTextDialog::GraphicTextDialog(QWidget *parent, const char *name)
^
cc1plus: warning: unrecognized command line option ‘-Wno-deprecated-register’

https://github.com/guitorri/qucs/blob/9b6f495fb46d0fa383841607ba82b1f14cee20d2/qucs/qucs/paintings/graphictextdialog.cpp#L34

It was here before this PR, but, now you've opened this PR, I guess it would be a good idea to remove that...

I think the PR works as expected. I tried to resize the window with different aspect ratios and it always triggers the color picker regardless the area you click inside the button. Moreover, I didn't see any warning in the standard output when clicking the button, so it seems to be ok 👍

diagramdialog_pr672

@guitorri
Copy link
Member Author

Thank you. I removed the unused variable you pointed and another one i found along the way.
Merging.

@guitorri guitorri merged commit 088e591 into Qucs:develop Oct 10, 2017
@guitorri guitorri deleted the push_button_color branch October 10, 2017 18:28
@andresmmera andresmmera mentioned this pull request Dec 19, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.0.20
Development

Successfully merging this pull request may close these issues.
3 participants
@guitorri @in3otd @andresmmera