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

Pyface Color Class and Trait Type #534

Closed
wants to merge 18 commits into from
Closed

Conversation

corranwebster
Copy link
Contributor

@corranwebster corranwebster commented Jun 6, 2020

This builds on #520 to create a Color class and corresponding PyfaceColor trait to represent a color in a toolkit independent way. It also includes a ColorDialog class built on top of these classes.

Together with #520 this resolves #476.

This should not be merged until #520 is merged. This requires features from Traits 6.1.

@corranwebster corranwebster reopened this Jun 12, 2020
@corranwebster corranwebster marked this pull request as ready for review June 12, 2020 11:01
Copy link
Contributor

@ievacerny ievacerny left a comment

Choose a reason for hiding this comment

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

Most of this looks good. There's potentially a bug with wx dialog and then some minor comments re typos and missing tests.

docs/source/trait_types.rst Outdated Show resolved Hide resolved
docs/source/trait_types.rst Outdated Show resolved Hide resolved
pyface/tests/test_color.py Outdated Show resolved Hide resolved
pyface/ui/null/color.py Outdated Show resolved Hide resolved
pyface/color_dialog.py Show resolved Hide resolved
pyface/tests/test_color.py Show resolved Hide resolved
pyface/ui/wx/color_dialog.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2020

Codecov Report

Merging #534 into master will increase coverage by 2.03%.
The diff coverage is 85.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #534      +/-   ##
==========================================
+ Coverage   37.08%   39.11%   +2.03%     
==========================================
  Files         470      485      +15     
  Lines       26027    26788     +761     
  Branches     3961     4075     +114     
==========================================
+ Hits         9652    10479     +827     
+ Misses      15948    15861      -87     
- Partials      427      448      +21     
Impacted Files Coverage Δ
pyface/__init__.py 82.60% <ø> (ø)
pyface/ui/null/color.py 0.00% <0.00%> (ø)
pyface/ui/qt4/init.py 62.50% <33.33%> (ø)
pyface/font_dialog.py 50.00% <50.00%> (ø)
pyface/ui/qt4/workbench/split_tab_widget.py 12.47% <50.00%> (ø)
pyface/ui/wx/color_dialog.py 51.85% <51.85%> (ø)
pyface/ui/wx/font_dialog.py 54.54% <54.54%> (ø)
pyface/qt/QtGui.py 81.48% <58.33%> (-9.00%) ⬇️
pyface/qt/QtNetwork.py 75.00% <60.00%> (ø)
pyface/qt/QtOpenGL.py 75.00% <60.00%> (ø)
... and 63 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c98d1d3...3f09729. Read the comment docs.

@corranwebster
Copy link
Contributor Author

Pending CI passing, this should be ready for re-review.

Copy link
Contributor

@ievacerny ievacerny left a comment

Choose a reason for hiding this comment

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

Qt dialog now works well, but wx dialog still doesn't

pyface/ui/wx/color_dialog.py Outdated Show resolved Hide resolved
@corranwebster
Copy link
Contributor Author

I think this is ready for one more go-around with review.

Copy link
Contributor

@ievacerny ievacerny left a comment

Choose a reason for hiding this comment

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

I'm still having issues with wx dialog window freezing, but I don't think it's related to this PR. I'm seeing the same behaviour with other wx dialogs as well (using both 4.0.7 and 4.1.0)

Another thing is that wx dialogs for me appear without "OK" or "Close" buttons, the only way to close the dialog is via system's "x" button, which behaves differently between qt and wx (see in-code comment). The same dialog without buttons appears using TraitsUI color editor, so again, I don't think it is related to this PR specifically, but maybe it's something that can be fixed here.
Edit: Here's the issue I opened for ColorEditor_demo.py enthought/traitsui#946. I get the same dialog using get_color function from this PR.

Note: if nobody else is seeing these issues, it might be that the state of GUI machinery might be messed up on my computer. In that case please ignore the comments.

Comment on lines +40 to +41
if result == OK:
return dialog.color
Copy link
Contributor

Choose a reason for hiding this comment

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

Closing the dialog via system's "x" button returns the color that was passed to this function when using wx. With qt it returns None as expected.

@corranwebster
Copy link
Contributor Author

Everything in here has now been pulled out into a separate PR.

@rahulporuri rahulporuri deleted the feat/color-trait branch October 27, 2020 17:35
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.

Basic Color and Font traits in Pyface
3 participants