-
-
Notifications
You must be signed in to change notification settings - Fork 682
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
Modularize tests buttons module #4740
base: main
Are you sure you want to change the base?
Conversation
|
4d1eb6c
to
0ebb661
Compare
0ebb661
to
ebe1c59
Compare
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.
LGTM, thanks for the refactoring!
Added one suggestion and a few nits.
def grid34(x: int, y: int) -> Coords: | ||
return grid(DISPLAY_WIDTH, 3, x), grid(DISPLAY_HEIGHT, 4, y) | ||
def grid34(x: int, y: int, layout_type: LayoutType) -> Coords: | ||
assert layout_type in (LayoutType.Bolt, LayoutType.Delizia) |
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.
nit: can be removed, since it is also checked in display_width()
& display_height()
.
(here and below)
|
||
|
||
# Horizontal coordinates | ||
def left(layout_type: LayoutType): |
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.
nit: add return type annotation?
(here and below)
def grid(dim: int, grid_cells: int, cell: int) -> int: | ||
step = dim // grid_cells | ||
ofs = step // 2 | ||
return cell * step + ofs | ||
|
||
|
||
def grid35(x: int, y: int) -> Coords: | ||
return grid(DISPLAY_WIDTH, 3, x), grid(DISPLAY_HEIGHT, 5, y) | ||
def grid35(x: int, y: int, layout_type: LayoutType) -> Coords: |
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.
Maybe it would simpler to add a "Buttons" class, that will have all the functions here as its methods, e.g.:
class Buttons:
def __init__(self, layout_type: LayoutType):
self.width = display_height(layout_type)
self.height = display_height(layout_type)
def grid35(self, x: int, y: int) -> Coords:
return grid(self.width, 3, x), grid(self.height, 5, y)
...
def info(self) -> Coords:
return (self.mid(), self.bottom())
...
So that you could do something like:
buttons = Buttons(debug.layout_type) # can be done once
debug.click(buttons.cancel()) # no need to pass layout type :)
WDYT?
This PR prepares
buttons
test module forEckhart
layout support that has a different screen resolution and different buttons positions. This is done by creating functions withlayout_type
param instead of using constants.On top of that, it refactors
set_selection
function to accommodate to this change.