Adding background_color #35

FoamyGuy · 2020-02-15T00:40:40Z

This change allows you to set background_color of the label in the constructor or with a property. Setting background_color to false, or excluding it entirely will result in same current behavior which is transparent background. This is referenced in issue #33.

update to master

adafruit_display_text/label.py

makermelissa · 2020-02-17T23:14:46Z

Hi, looks mostly good. However, if you don't set the background color so that it is transparent and read the property, it returns 0, which is the same value returned if it is black. Can we modify this so that if the color is transparent, it returns None?

It doesn't appear circuitpython currently has a native way to check whether it is transparent, so we may need to store that as a flag on the label internally.

caternuson · 2020-02-17T23:41:46Z

adafruit_display_text/label.py

@@ -57,7 +57,7 @@ class Label(displayio.Group):
       :param int color: Color of all text in RGB hex
       :param double line_spacing: Line spacing of text to display"""
    def __init__(self, font, *, x=0, y=0, text=None, max_glyphs=None, color=0xffffff,
-                 line_spacing=1.25, **kwargs):
+                 background_color=False, line_spacing=1.25, **kwargs):


Can you change the default to None instead and then change your logic to match? You can check for None easily:

if background_color: # action if there is a color set else: # action if it's None

caternuson · 2020-02-17T23:43:08Z

examples/display_text_background_color.py

+text_area = label.Label(terminalio.FONT, text=text, color=0x0000FF, background_color=0xFFAA00)
+text_area.x = 10
+text_area.y = 10
+board.DISPLAY.show(text_area)


Also add some lines that show:

querying current background color

changing back to no background color

caternuson · 2020-02-17T23:44:47Z

adafruit_display_text/label.py

+    @property
+    def background_color(self):
+        """Color of the background as an RGB hex number."""
+        return self.palette[0]


It would be good if this would return None when there is no background color set.

…arency and return None when background is transparent. add more functionality to example.

FoamyGuy · 2020-02-18T01:49:45Z

@makermelissa @caternuson I believe all of the mentioned changes have been made with the new commits. In making and testing the changes I also found and fixed a few issues with changing too and from transparent background colors.

makermelissa · 2020-02-18T02:56:21Z

Thanks @FoamyGuy. One more small thing. Can we change it so self.transparent_background is private so that it becomes self._transparent_background. That way you don't have users doing something silly like:

text_area = label.Label(terminalio.FONT, text="hello", color=0x0000FF)
text_area.transparent_background = False
print(text_area.background_color)

FoamyGuy · 2020-02-18T03:30:53Z

@makermelissa Got it done. Thank you.

caternuson · 2020-02-18T03:38:34Z

adafruit_display_text/label.py

+            self.palette.make_transparent(0)
+        else:
+            self.transparent_background = False
+            self.palette[0] = background_color


Sorry, I totally forgot about needing to accept 0 as a possible valid color. If that were done with above, then it would be treated as setting transparent. How about this?

if background_color is not None: self.palette[0] = background_color self.palette.make_opaque(0) self.transparent_background = False else: self.palette.make_transparent(0) self.transparent_background = True

Or could maybe do like you did in the setter?

(but also with the leading underscore)

Ah right, thank you. just pushed some commits to fix this in the constructor.

makermelissa · 2020-02-18T04:18:00Z

adafruit_display_text/label.py

@@ -71,7 +71,14 @@ def __init__(self, font, *, x=0, y=0, text=None, max_glyphs=None, color=0xffffff
        self.y = y

        self.palette = displayio.Palette(2)
-        self.palette.make_transparent(0)
+        if background_color or background_color == 0x000000:


This might be better expressed as: if background_color is not None:

makermelissa · 2020-02-18T04:18:30Z

adafruit_display_text/label.py

+
+    @background_color.setter
+    def background_color(self, new_color):
+        if new_color or new_color == 0x000000:


same with this: if new_color is not None:

makermelissa · 2020-02-18T04:19:15Z

Thank you for making these changes. We want your code to be in its best possible form.

FoamyGuy · 2020-02-18T15:12:22Z

@makermelissa there is a new commit now that changes both if statements to use the is not None syntax. I don't foresee any issues with it, but I won't be able to test the new commit specifically on a device until later tonight.

I really appreciate everyone taking the time to point out ways it can be improved. I cut my teeth as a solo programmer, and even now I work on a team of only 2-3 developers. So feedback on source code is something I haven't got much experience with before getting involved with Circuit Python. I appreciate the feedback because I can hopefully learn and improve from it.

caternuson · 2020-02-18T15:40:56Z

@FoamyGuy Thanks for making all the changes.

@makermelissa At this point - looks OK to me.

makermelissa

Looks good to me too. Thanks @FoamyGuy!

Updating https://github.com/adafruit/Adafruit_CircuitPython_BusDevice to 4.2.1 from 4.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_BusDevice#45 from dhalbert/fix-end-values Updating https://github.com/adafruit/Adafruit_CircuitPython_Display_Text to 2.4.0 from 2.3.2: > Merge pull request adafruit/Adafruit_CircuitPython_Display_Text#35 from FoamyGuy/master Updating https://github.com/adafruit/Adafruit_CircuitPython_PyBadger to 1.1.1 from 1.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_PyBadger#20 from nnja/fix_business_card_display

FoamyGuy added 4 commits February 14, 2020 18:10

Merge pull request #1 from adafruit/master

2086944

update to master

adding background_color and usage example

cefd454

undo ide formatting changes

39431ec

adding getter setter properties for background_color

eae4cb4

FoamyGuy requested a review from a team February 15, 2020 00:44

FoamyGuy linked an issue Feb 15, 2020 that may be closed by this pull request

Label could have a background color #33

Closed

makermelissa reviewed Feb 15, 2020

View reviewed changes

adafruit_display_text/label.py Outdated Show resolved Hide resolved

FoamyGuy added 2 commits February 14, 2020 20:22

fix typo

7868b40

fix typo in background_color example

fb70ae4

FoamyGuy requested a review from a team February 17, 2020 15:02

caternuson requested changes Feb 17, 2020

View reviewed changes

FoamyGuy added 3 commits February 17, 2020 19:32

fix setting background back to transparent. add logic to track transp…

5dfc291

…arency and return None when background is transparent. add more functionality to example.

remove unneeded else block

80ebaa9

fix for setting background to visible black

46b04fe

add leading underscore to _transparent_background

870643a

caternuson requested changes Feb 18, 2020

View reviewed changes

FoamyGuy added 2 commits February 17, 2020 21:44

fix constructor for visible black background color

0fe25b5

fix constructor for visible black background color

4d65e55

makermelissa reviewed Feb 18, 2020

View reviewed changes

fix constructor for visible black background color

72c5b62

caternuson approved these changes Feb 18, 2020

View reviewed changes

makermelissa approved these changes Feb 18, 2020

View reviewed changes

makermelissa merged commit b506a0a into adafruit:master Feb 18, 2020

Adding background_color #35

Adding background_color #35

Uh oh!

Conversation

FoamyGuy commented Feb 15, 2020

Uh oh!

Uh oh!

makermelissa commented Feb 17, 2020

Uh oh!

caternuson Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

caternuson Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

caternuson Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

FoamyGuy commented Feb 18, 2020

Uh oh!

makermelissa commented Feb 18, 2020

Uh oh!

FoamyGuy commented Feb 18, 2020

Uh oh!

caternuson Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

caternuson Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

FoamyGuy Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

makermelissa Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

makermelissa Feb 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

makermelissa commented Feb 18, 2020

Uh oh!

FoamyGuy commented Feb 18, 2020

Uh oh!

caternuson commented Feb 18, 2020

Uh oh!

makermelissa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

makermelissa Feb 18, 2020 •

edited

Loading