Skip to content

Anchoring Point relative to the Baseline #134

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

Merged
merged 8 commits into from
Mar 14, 2021
Merged

Anchoring Point relative to the Baseline #134

merged 8 commits into from
Mar 14, 2021

Conversation

jposada202020
Copy link
Contributor

This replace PR #124. Will solve isssue #100.

Logic was tested in Previous PR. It was tested again using the blinka display for label and bitmap label

These are the results after refactoring. Using the same code presented in PR #124

image

image.

@FoamyGuy Please review with code in previous PR. Thanks :)

@jposada202020
Copy link
Contributor Author

This also include replacement for PR #123

@kmatch98
Copy link
Contributor

@jposada202020 this is looking really good.

I noticed errors in the doc strings in the last parameters. I think you have an extra : after param that is making it weird. It's the same issue on both label and bitmap_label.

This is a great addition, thanks again for all your work on this.

On label:
image

On bitmap_label:
image

@jposada202020
Copy link
Contributor Author

@kmatch98 Thanks for the review and comments, I will work on those changes,You are on point there are two ":"
did you generate the document using sphinx?
I just want to make sure I am verifying that everythng is ok when correcting those changes

@kmatch98
Copy link
Contributor

kmatch98 commented Mar 10, 2021

I didn't build it locally, I just noticed it on the "official" released docs.

If you've never build Sphinx documents, you can setup the build the Sphinx docs locally and verify them (here's the guide).

Go into the docs folder and then the main command to build them is sphinx-build -E -W -b html . _build/html.

And I struggle a lot with Sphinx, it's formatting requirements are really strange to me. It matters exactly how you wrap lines (I think there may need an extra space when you wrap lines). And it matters exactly what column you start your line relative to the three " marks that start and end the section. I usually go from an example that works and try and copy it, but often I still struggle to figure out how to line everything up to make Sphinx do what I want it to.

@jposada202020
Copy link
Contributor Author

Thanks for the Infor I will check that out!

@jposada202020
Copy link
Contributor Author

Corrections in this commit f6d19be

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Thanks for this great new feature @jposada202020!

I tested these changes successfully (both .py and .mpy versions) on:

Adafruit CircuitPython 6.2.0-beta.3-36-g103632f44 on 2021-03-07; Adafruit PyPortal with samd51j2

as well as Blinka_Displayio using the supplied test script from the linked older PR.

These changes look good to me. the baseline value for anchor_point feature is a really nice addition. Thanks for adding the type hints as well, this will be a big help in pycharm.

I made one extra tiny commit to remove the extra colons mentioned in the doc strings. I think that commit you linked was to a different branch or something. I fixed those up and checked the sphinx docs output and it's looking nice now to me.

@FoamyGuy FoamyGuy merged commit e97e698 into adafruit:master Mar 14, 2021
@jposada202020
Copy link
Contributor Author

@FoamyGuy Thanks, yes I noticed that, the commit is in the Vertical Label branch, Thank you for the review and made the changes.

@jposada202020 jposada202020 deleted the tab_replacement branch March 14, 2021 04:38
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 14, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.16.0 from 2.15.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#133 from FoamyGuy/new_font
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#134 from jposada202020/tab_replacement
  > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#142 from flavio-fernandes/fix.init.wout.text

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_Layout to 1.2.1 from 1.1.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#10 from kmatch98/switch_round
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_Layout#12 from FoamyGuy/adding_icon_widget
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