Initial code #2
Initial code #2
Conversation
|
This now has the initial code for grid_layout and a few minimal examples. I think it's ready to go at least as a starting point |
There was a problem hiding this comment.
This is great! Thank you for getting this going.
I have some inline suggestions, and will probably have more, but this is a start. A lot of it relates to the concept of a view still being unclear to me. Bear in mind, this is only a set of suggestions, and if you believe we should stick with the current nomenclature, I will support that as long as it's well documented.
The display_layout_simpletest.py file should not be empty. I would suggest for now making it identical to the gridlayout_simpletest and as we add more to this library, include the features in the simpletest example, which is to say, it would test all the features of the library. You can include a docstring with that explanation at the beginning of the file so you and others who contribute will remember/know to do that.
|
I will work on these and push changes tomorrow. |
|
|
||
| repos: | ||
| - repo: https://github.com/python/black | ||
| rev: stable |
| hooks: | ||
| - id: black | ||
| - repo: https://github.com/fsfe/reuse-tool | ||
| rev: latest |
There was a problem hiding this comment.
This will need to change to v0.12.1
Thank you! I'll get those in tonight along with the other changes. |
|
@FoamyGuy Hold off on the pre-commit changes, we're looking into those. Should have an answer soon. |
|
@FoamyGuy Thanks for replying! I'm glad my suggestions work for you. Looking forward to the updates! I'll test it once you get it settled. |
|
I think this is all set now, pending the potential pre-commit changes. I did go ahead and put a copy of the grid_layout example into the main simpletest script. We can add to that as we get other layouts and widgets in this library. @kattni I believe it should be functioning the same as the copy you started with, but would not be an actual drop-in replacement due to the changed names of some functions, arguments, and properties. |
There was a problem hiding this comment.
Examples
Two of the examples look to be almost the same, or am I missing something?
examples/displayio_layout_gridlayout_simpletest.pyexamples/displayio_layout_simpletest.py
Resizing and centering widgets in cells (plus a rabbit hole of label and bitmap_label)
One thing that I realized is that some Widgets' width and height may have constraints on them. I suggest that all Widgets be resized using tuples (width, height) so that the Widget can decide on its own aspect ratio.
On a related note, a label or bitmap_label dimenions cannot actually be modified. Therefore, if grid_layout tries to resize a label, the label.width and label.height should actually not change.
That brings me to the next point, that I think grid_layout should default to centering the Widget in the cell.
But there are problems in label and bitmap_label libraries that need to be resolved to allow this.
For the simpletest example, it plots four labels, but they are all in the upper left corner of the cell. I think the default should be that items inside of a cell should be centered. In looking deeper at the label structure, the label.width and label.width actually mean something else in the label Class. Instead, currently the bounding_box is where the actual pixel dimensions of the label are kept. (Currently label.width is the number of characters and label.height is the pixel height of the text.)
Also, another related issue that will arise is that bitmap_label does not have any .height or .width property.
I think if we define that Widget's should have properties of height and width, then we'll have to go change label and bitmap_label to accommodate this.
Anyway, I think that grid_layout placement method should be modified so that it places the x,y locations of the Widget and centers it in the cell. That will require the _layout_cells function to place the x position something like this:
cell["content"].x = (
int(grid_position_x * self._width / grid_size_x) + self.cell_padding
+ int( (self._width/grid_size_x) - cell["content"].width) /2 )
)
cell["content"].y = (
int(grid_position_y * self._width / grid_size_y) + self.cell_padding
+ int( (self._height/grid_size_y) - cell["content"].height) /2 )
)Summary
- Update
grid_layoutto center items in cells by checking the item's actual width and height. - Update
labelso theheightandwidthmean whatgrid_layoutwants - Add
heightandwidthgetters/setters tobitmap_label
I didn't get around to checking buttons.....
|
@kmatch98 Thank you for taking the time to go over this and review. Especially catching the bad arguments in the examples! It true that I decided to make a new kind of label instead of changing the existing ones. I think the way the existing ones work is nice as well because they can grow to fit whatever text you want to put inside which is desirable sometimes. It could perhaps be modified to work both ways within the same class but the code would get more complex and larger to import. I think right now I'm in favor of leaving the existing ones alone and encouraging people to use this new type of label for best support with grid_layout. The new one started as a copy of bitmap_label and I changed up the parts necessary for it to have However I do still want to have some decent support for the existing bitmap_label and label and more generally speaking widgets that don't allow size manipulation so I do also like your idea of centering widgets within the cell. I'd love to ultimately see a configuration option that allows aligning widgets smaller than the cell to either left, center, or right. I will start with centering for now but look to add that enhancement sometime after this initial version code is in. With regards to setting size via tuple I'm not opposed, but I do like the ability to change one without worrying about the other that separate properties allows. I'm not sure if I understand if or why the tuple is required for widgets to maintain their own aspect ratio. Couldn't similar logic live in the height and width properties? so whenever user changes one of them it will adjust it's internal "actual drawn size" (which is clamped to the aspect ratio desired) to fit within the new bounds based on whichever property the user changed. |
|
Here’s some clarification on my proposal to set width and height in a tuple. Imagine a widget that likes to be in a ratio of 3:1width to height. When it’s first created it is 30 wide, 10 high. Assuming that the widget always stays inside the maximum size it can within width height, then it keeps an actual size of 30 x 10. Next we say to change height to 30. So does it need to remember that it’s actual height and width is currently 30x10 but the “last_requested” width was 90? I think that will cause way to much trouble to keep some memory of what was requested. Instead I propose that you send a tuple of (width, height) and the widget will decide how to best fit it into that maximum boundary. In contrast, if we set one at a time I think we can expect to have some weird behavior depending upon the sequence of width and height requests. I do think Widgets can keep separate width and height setter functions, but for use with grid_layout I think the each shape should be set with a tuple. And even better (if we don’t want to be too strict with the Widget class definition), we can have grid_layout try using the tuple first if the Widget tuple function is available, and if not it can fall back to individual width and height setters. Hope I didn’t muddy the waters any worse. I really think grid_layout is going to be useful for anyone trying to make a quick GUI. |
|
I do see what you mean about setting the sizes individually causing potential weird behavior. In the Resizeable button PR I've added a In this PR the latest commit makes use of that new resize function if the content object has it, but still falls back to width and height properties if not. |
|
I tested the |
Now example adafruit#1 displays the temperature in degrees Celsius while example adafruit#2 displays the temperature in degrees Fahrenheit.
Right now it's got the cookie-cutter files, the grid_layout, and everything needed to pass CI.
Still needs at least one example. I think it would also be good to maybe hold off releasing this library (or grid_layout at least) until after a few changes are merged to
display_buttonanddisplay_textthat will make both of them work better with this library to alleviate some of the mentioned in #1