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

Fix reset text scale on item unhighlight #768

Closed

Conversation

@fireclawthefox
Copy link
Contributor

fireclawthefox commented Nov 1, 2019

Set text scale to previous unhighlighted scale on unhighlight to keep custom scales

Set text scale to previous unhighlighted scale on unhighlight to keep
custom scales
@rdb

This comment has been minimized.

Copy link
Member

rdb commented Nov 2, 2019

Would you be willing to show some code demonstrating the bug, or better yet, add a unit test? That will allow me to merge this into the next release with confidence, thanks!

Unittest for most DirectOptionMenu functions
including item_text_scale changes on un-/highlighting
@fireclawthefox

This comment has been minimized.

Copy link
Contributor Author

fireclawthefox commented Nov 2, 2019

Added unittests for DirectOptionMenu. The test for the text scale behavior is at the bottom.

@rdb rdb added this to the 1.10.5 milestone Nov 3, 2019
@rdb rdb self-assigned this Nov 3, 2019
@rdb
rdb approved these changes Nov 3, 2019
Copy link
Member

rdb left a comment

Thanks! Will merge shortly (the only change I'm making is changing the variable name to self._prevItemTextScale, since there's no good reason to mark this as a public property, I think.)

except:
# Showing an option menu without items will raise an exception
exceptionThrown = True
assert exceptionThrown

This comment has been minimized.

Copy link
@rdb

rdb Nov 3, 2019

Member

I'll fix this for now, but for future reference, pytest has a more idiomatic way to do this:

with pytest.raises(Exception):
    menu.showPopupMenu()
@rdb

This comment has been minimized.

Copy link
Member

rdb commented Nov 3, 2019

Thank you, committed to 1.10.5 branch in eb3b45e and e6fc6c6.

I've fixed the "TODO" in your unit test by allowing DirectOptionMenu to be used without ShowBase, in 69e8b4e and 2e3b350.

@rdb rdb closed this Nov 3, 2019
@fireclawthefox

This comment has been minimized.

Copy link
Contributor Author

fireclawthefox commented Nov 3, 2019

Thanks for the hint with the pytest exception test and fixing the todo. Changing the variable is fine, I don't know of a good reason for it to be public either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.