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

DataView checkboxes #694

Merged
merged 5 commits into from
Sep 23, 2020
Merged

DataView checkboxes #694

merged 5 commits into from
Sep 23, 2020

Conversation

corranwebster
Copy link
Contributor

@corranwebster corranwebster commented Sep 22, 2020

This PR adds support for checkboxes in the data view; and adds a bool value type that uses them.

The bool value type uses a different code path than the editable values because the checkboxes are always interactable and don't require a click to activate an editor.

@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2020

Codecov Report

Merging #694 into master will increase coverage by 0.11%.
The diff coverage is 70.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
+ Coverage   39.72%   39.84%   +0.11%     
==========================================
  Files         492      493       +1     
  Lines       27222    27259      +37     
  Branches     4134     4140       +6     
==========================================
+ Hits        10815    10862      +47     
+ Misses      15935    15930       -5     
+ Partials      472      467       -5     
Impacted Files Coverage Δ
pyface/ui/qt4/heading_text.py 90.47% <ø> (-0.83%) ⬇️
pyface/ui/qt4/data_view/data_view_item_model.py 50.00% <37.50%> (-1.58%) ⬇️
pyface/data_view/value_types/bool_value.py 85.71% <85.71%> (ø)
pyface/data_view/abstract_value_type.py 97.43% <100.00%> (+0.88%) ⬆️
pyface/data_view/value_types/api.py 100.00% <100.00%> (ø)
pyface/ui/qt4/console/console_widget.py 29.28% <0.00%> (+0.20%) ⬆️
pyface/ui/qt4/code_editor/code_widget.py 44.16% <0.00%> (+0.84%) ⬆️
pyface/wx/python_stc.py 10.36% <0.00%> (+1.21%) ⬆️
pyface/ui/wx/window.py 77.10% <0.00%> (+2.40%) ⬆️
... and 4 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 f9b1349...01fcf13. Read the comment docs.

pyface/data_view/abstract_value_type.py Outdated Show resolved Hide resolved
pyface/data_view/abstract_value_type.py Outdated Show resolved Hide resolved
Comment on lines +279 to +283
return (
CheckState.CHECKED
if model.get_value(row, column)
else CheckState.UNCHECKED
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why the default get_check_state doesn't return a DataViewGetError like set_check_state does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a default implementation that is almost always what you want: if the value has a check state, then it should be checked if the underlying value is truthy and unchecked if it is falsey.

By implementing it on the ABC we save repetition elsewhere (we do similar things for get text and get image, try to have a sane default for the getter).

pyface/data_view/value_types/bool_value.py Outdated Show resolved Hide resolved
pyface/data_view/value_types/bool_value.py Outdated Show resolved Hide resolved
@corranwebster
Copy link
Contributor Author

I think that's everything.

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM if CI is also happy.

@corranwebster corranwebster merged commit 7f31070 into master Sep 23, 2020
@corranwebster corranwebster deleted the feat/data-view-checkbox branch September 23, 2020 14:52
@corranwebster
Copy link
Contributor Author

corranwebster commented Sep 25, 2020

Note: Partially resolves #533

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.

3 participants