-
Notifications
You must be signed in to change notification settings - Fork 39
Bitmap label base alignment #120
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
Bitmap label base alignment #120
Conversation
# Conflicts: # adafruit_display_text/label.py
Wow this looks really great and thanks fro getting this done so promptly! I’m actually right in the middle of something that needs this exact function so I’m eager to try it out. I had some unexpected guests arrive today (housing some friends that los power) so it may take me a day or two to put this through its paces. So my apologies if it takes some time for me to try it out and get you any constructive feedback. |
1 similar comment
Wow this looks really great and thanks fro getting this done so promptly! I’m actually right in the middle of something that needs this exact function so I’m eager to try it out. I had some unexpected guests arrive today (housing some friends that los power) so it may take me a day or two to put this through its paces. So my apologies if it takes some time for me to try it out and get you any constructive feedback. |
@jposada202020 This looks great to me. And thanks for the detailed review of typos in the comments. I tried the example and then some other code I'm working on and the I recommend that we change This looks good to me. Thanks for the making these changes, this is a helpful addition to both these libraries. |
@kmatch98 Thanks for the quick answer, revision and effort put into this. For the default to be True, I have not issue with that. It would change from revision to version to let people know that the behavior will change as the label/bitmap_label will not longer center around the midpoint, this will imply also changing the Learning Guides. Both, I am ok with both, redoing the PR to do that change in both libraries it would not represent a lot of work. |
I think I lean toward defaulting it to I do think new way is nicer, so I can see the appeal of defaulting to that. But it would cause any existing code to start placing things differently so folks would have to go back and either add the parameter and set it I'm open to input from others as well though, I do admit I tend to go toward not changing defaults more often than not. Sometimes I overestimate how much trouble the change will actually cause in the end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this successfully on Adafruit CircuitPython 6.2.0-beta.2 on 2021-02-11; Adafruit PyPortal with samd51j20
Looks good to me. Thank you @jposada202020!
Updating https://github.com/adafruit/Adafruit_CircuitPython_datetime to 1.1.0 from 1.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_datetime#2 from makermelissa/master Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.14.0 from 2.12.4: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#120 from jposada202020/bitmap_label_base_alignment > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#115 from jposada202020/base_alignement_proposal
There present PR is to include new parameter base_alignment to the bitmap_label. This parameter will allow to align to different bitmap_label along the baseline. This is similar to the work done on the label library.
Including in this text the code used to test. Also included a Picture of the results.
As you can see in the picture, including this parameter will allow the user to align both the Label and the bitmap_label at the same time. If a sample code is needed let me know. I consider that the example for the base_label parameter in label is enough.
@kmatch98 if you can take a look at this, I am guessing that you are the writer of this library. So any input would be appreciated.
And sorry again for the fonts :)
Test code @FoamyGuy I know my test are complicated ;)