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

Resolving icon flaw in comboBox2D #576

Closed
wants to merge 4 commits into from

Conversation

lej0hn
Copy link
Contributor

@lej0hn lej0hn commented Apr 18, 2022

Fixing bug mentioned in #562

@pep8speaks
Copy link

pep8speaks commented Apr 18, 2022

Hello @lej0hn! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2022-04-19 17:01:37 UTC

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @lej0hn,

Thank you for doing this!

Can you address all the pep8 issues and add a test?

Thank you

@lej0hn
Copy link
Contributor Author

lej0hn commented Apr 19, 2022

Hi @skoudoro ,
Pep8 issues have been resolved and the test is coming soon!

@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #576 (001c492) into master (a7c31ae) will increase coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #576      +/-   ##
==========================================
+ Coverage   88.60%   88.87%   +0.26%     
==========================================
  Files          55       55              
  Lines       10957    11375     +418     
  Branches     1081     1122      +41     
==========================================
+ Hits         9708    10109     +401     
- Misses        954      965      +11     
- Partials      295      301       +6     
Impacted Files Coverage Δ
fury/fury/data/fetcher.py 71.03% <0.00%> (-1.38%) ⬇️
fury/fury/layout.py 98.63% <0.00%> (-1.37%) ⬇️
fury/fury/tests/test_utils.py 94.26% <0.00%> (-0.74%) ⬇️
fury/fury/actor.py 88.15% <0.00%> (-0.17%) ⬇️
fury/fury/ui/elements.py 87.21% <0.00%> (-0.09%) ⬇️
fury/fury/lib.py 100.00% <0.00%> (ø)
fury/fury/ui/helpers.py 98.21% <0.00%> (ø)
fury/fury/tests/test_layout.py 100.00% <0.00%> (ø)
fury/fury/tests/test_material.py 100.00% <0.00%> (ø)
fury/fury/tests/test_primitive.py 100.00% <0.00%> (ø)
... and 6 more

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @lej0hn,

This workaround works but I think it would have been great to see why all callbacks are called so many times instead of adding a flag.

Also, Can you add a test? Thanks

@skoudoro
Copy link
Contributor

superseded by #768, closing

@skoudoro skoudoro closed this Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants