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

Added "cx" and "cy" property to the label to change center of Label #22

Merged
merged 10 commits into from
Jan 28, 2020

Conversation

neoxharsh
Copy link
Contributor

I have added "cx" and "cy" property to the label, such that setting those will move the center of the text rather than the top left corner of the text. My apologies if it is already available.

Eg. for PyPortal

timeDisplay = label.Label(font,text='12:60:60',color=color)
timeDisplay.cx = 160 
timeDisplay.cy = 120

@tannewt
Copy link
Member

tannewt commented Nov 2, 2019

I like how you are using the Group's x and y to handle this! I'd rather not add separate cx and cy properties though. Instead, I'd like to see one or two properties that set the anchor position. Right how it's fixed as Center Left but you'd like Center Center.

With the anchor property, changes to the bounding box, x, or y (overridden) would trigger changing the Group's x and y.

Thanks!

@kattni
Copy link
Contributor

kattni commented Jan 8, 2020

@neoxharsh Thank you for the contribution! There are a number of linting errors that need addressing, as well as a change request in the comment above. If you're still interested in seeing this included in this library, please make the necessary changes. If you need assistance with any of it, please let us know. Thanks!

moving to anchor point instead of cx and cy
fix for missing _anchor_point in init
_anchored_position = (
self.x-self._boundingbox[2]*self._anchor_point[0],
self.y-self._boundingbox[3]*self._anchor_point[1])
return _anchored_position
Copy link
Contributor

Choose a reason for hiding this comment

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

You can avoid creating _anchored_position and shorten this up to something like:

return self.x-self._boundingbox[2]*self._anchor_point[0],
       self.y-self._boundingbox[3]*self._anchor_point[1]

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. Thank you, this way it won't need to create the variable at all.

@FoamyGuy
Copy link
Contributor

Here is an example script that shows usage and testing of the anchor point functionality.

On a PyPortal it will output this to the display:
anchor_point_text_example

removing unneeded _anchored_position variable.
adding _anchor_point into init
@FoamyGuy
Copy link
Contributor

@caternuson I am trying to work through the pylint issues. It looks like it's mad that I've used self.x = ... and self.y = ... in the new function and they aren't defined in the init. They would be coming from the super class displayio.Group in this case. I could initialize them to 0 in the init() here to fix the pylint error I think, but I'm not sure if that would cause trouble.

@caternuson
Copy link
Contributor

Hmmm...I think that has to do with the linter not knowing the actual class layout for things from the CP firmware. @tannewt What say you?

@makermelissa
Copy link
Contributor

makermelissa commented Jan 28, 2020

Perhaps the linter would be happy if you set the x and y to some default value inside of the __init__ function.

tested with simpletest and the anchor point test linked above.
@neoxharsh
Copy link
Contributor Author

@tannewt @kattni @makermelissa @FoamyGuy @caternuson, thanks for all your support. I am on vacation, staying away from tech for a while. 😇

@kattni
Copy link
Contributor

kattni commented Jan 28, 2020

@makermelissa @caternuson @FoamyGuy Did one of you test this iteration? If so, we can merge it. If not, can one of you please test it? Thanks!

@kattni
Copy link
Contributor

kattni commented Jan 28, 2020

@neoxharsh No worries! Thank you for the original contribution! Have a wonderful vacation!

@makermelissa
Copy link
Contributor

@kattni, I did not test. I was merely interpreting the Pylint error.

@caternuson
Copy link
Contributor

@kattni I just tested the linked example above on a PyPortal and it worked as shown. Also ran display_text_simpletest.py example and it continues to work as expected.

@FoamyGuy Do you want to add the example you linked above to the examples folder as part of this PR?

@kattni
Copy link
Contributor

kattni commented Jan 28, 2020

@FoamyGuy Please create a new PR for the example.

@caternuson Thank you for testing! I'll get this merged.

Copy link
Contributor

@caternuson caternuson left a comment

Choose a reason for hiding this comment

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

That'll work. For future ref - you don't even need the parans to return a tuple. Can just do like:

return x, y, z

@kattni kattni merged commit 4d53a37 into adafruit:master Jan 28, 2020
@kattni
Copy link
Contributor

kattni commented Jan 28, 2020

@FoamyGuy At your convenience, please create a new PR with the example you linked. Please ping me for assistance with naming the example file if it is unclear what it should be named. Thanks!

@FoamyGuy
Copy link
Contributor

@kattni I did test last night with both the simpletest, and the example linked above.

@caternuson I had tried it without the parans first actually. If I understood correct I was getting an Indention error due to that return statement being split across two lines. Explicitly adding the parans seemed to fix the issue for me.

I will make a PR to add the example code today, it'll probably take me a bit to get to, but I'll definitely get it done at some point.

@caternuson
Copy link
Contributor

@FoamyGuy Ah, for multiple lines without parans, may have needed a \ continuation. Something like:

return x, \
       y

No biggie. Mainly just wanted to point out the syntax is possible.

@FoamyGuy
Copy link
Contributor

Ah, I see. Thank you. I appreciate it, I've come a long way with Python, but I'm still not very familiar with some of it's features.

@tannewt
Copy link
Member

tannewt commented Jan 28, 2020

@caternuson @FoamyGuy I prefer using parens to allow line wraps. I think it's easier to see than the trailing \

@caternuson
Copy link
Contributor

^^ yep. i'd agree. for simple, shorter, one liners, can use other syntax

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jan 30, 2020
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.

None yet

6 participants