Grid menu #123

derivmug · 2018-10-22T18:28:06Z

This branch adds a 3x3 grid menu.

CRImier

Pretty good, but would be better off based on BaseListBackgroundableUIElement (less code duplication and more features).

CRImier · 2018-10-22T18:38:46Z

ui/grid_menu.py

+		new_x = self.selected_option['x'] + x_mod
+		new_y = self.selected_option['y'] + y_mod
+
+		if new_x >= 1 and new_x <= 3:


3 is a magic number in this and next rows, care to replace with a const?

Suggested change

if new_x >= 1 and new_x <= 3:

if new_x >= 1 and new_x <= self.GRID_WIDTH:

if new_y >= 1 and new_y <= self.GRID_HEIGHT:

CRImier · 2018-10-22T18:41:32Z

ui/grid_menu.py

+	def refresh(self):
+		self.draw_menu()
+
+	def _move_cursor(self, x_mod, y_mod):


This lacks navigation when pressing RIGHT on the last column (or LEFT on first). Feel free to borrow the code from the calendar =)

or, if based on BaseListBackgroundableUIElement, its nav functions will work (but move_up/down will need to be defined as move_left/right and proper move_up/down will need to be added)

CRImier · 2018-10-22T18:43:41Z

ui/grid_menu.py

+
+class GridMenu(BaseUIElement):
+
+	GRID_WIDTH = 3


be able to define it as an init() argument? seems like a good base UI element that we could later move, say, DatePicker to, I will think about it more.

CRImier · 2018-10-22T18:46:48Z

ui/grid_menu.py

+from base_ui import BaseUIElement
+from canvas import Canvas
+
+class GridMenu(BaseUIElement):


Consider defining your own validate_contents(self, contents). If someone passes contents that isn't a widthxheight array of entries, I imagine it can break in subtle ways - better warn the developer about it first =D

or, if you base it off BaseListBackgroundableUIElement, its validate_contents will be more than suitable

CRImier · 2018-10-22T18:56:41Z

<CRImier> derivmug: what do you think about moving GridMenu to BaseListBackgroundableUIElement?
<derivmug[m]> Ok, what would I need to change/add
<CRImier> well, it's a ListUIElement
<CRImier> it has menu navigation primitives already in
[...]
<CRImier> as well as useful stuff for working with "contents"
<CRImier> come to think of it, we don't really need to pass a X*Y grid as "contents"
<CRImier> merely rendering it all as a list (and overriding move_up/down/left/right ofc)
<CRImier> should do
<CRImier> like you have with calendar, for example
<CRImier> actually
<CRImier> don't really need to redefine move_up/down/left/right?
<CRImier> oh wait, you do
<CRImier> move_left/right would work as move_up/down currently do
<CRImier> in list UI elements
<CRImier> and move_up/down would skip to next/previous row

codecov · 2018-10-22T19:51:42Z

Codecov Report

Merging #123 into devel will increase coverage by 0.07%.
The diff coverage is 34.95%.

@@            Coverage Diff             @@
##            devel     #123      +/-   ##
==========================================
+ Coverage   40.56%   40.64%   +0.07%     
==========================================
  Files         248      251       +3     
  Lines       19950    20276     +326     
==========================================
+ Hits         8093     8241     +148     
- Misses      11857    12035     +178

Impacted Files	Coverage Δ
apps/hardware_apps/zp_test/key_test.py	`0% <ø> (ø)`
emulator.py	`3.84% <0%> (-0.03%)`	⬇️
apps/hardware_apps/zp_test/main.py	`0% <0%> (ø)`
ui/canvas.py	`86.54% <100%> (+0.08%)`	⬆️
ui/__init__.py	`100% <100%> (ø)`	⬆️
ui/tests/test_base_list_ui.py	`75% <100%> (+2.5%)`	⬆️
ui/base_list_ui.py	`82.1% <100%> (+5.41%)`	⬆️
apps/example_apps/sandbox/main.py	`40.9% <16.66%> (-22.73%)`	⬇️
ui/tests/test_grid_menu.py	`32.19% <32.19%> (ø)`
apps/example_apps/grid_menu_test/main.py	`60% <60%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bac028e...c4021ba. Read the comment docs.

…throw an exception, for now)

…SB port testing function but haven't finished it yet

…t app changes, no time to split)

CRImier · 2018-11-27T16:51:09Z

Hi! Thank you for the work, currently building the main menu from it. Some feedback:

We do already have plenty of pointer handling code. So, redefining pointer to pretty much be x_index doesn't make a lot of sense and will introduce differences from code that we already have that relies on pointer to be an index in contents, and isn't even necessary.
There's some code and variables in the GridMenu that actually ought to be in GridView because it's only relevant when we display the menu. I.e. we only actually care about y_index when we draw the menu on the screen.

Working further - got an idea in my head of how it all could work, hope you like how it turns out =)

CRImier · 2018-11-27T16:56:15Z

Also, do check how other UI elements work, it doesn't make sense to break conventions - i.e. making contents into a keyword argument when Menu, a parent UI element, has it as the first positional argument.

…own, making sure they don't refresh more than once per call

…a view mixin. WIP - out-of-screen entry navigation doesn't yet work properly, might need to adjust first_displayed_entry better.

…also get added)

…emoving it out of pyavrdude because can't test it yet

…n't yet fully working

…n load)

…ased UI elements; adding tests for it

CRImier · 2018-12-23T15:26:03Z

@derivmug thank you!

derivmug added 5 commits October 22, 2018 12:57

Add base for grid_menu

48bb0cb

Add grid navigation

c2c725c

Draw app names

967f66c

test lambda for menu

02c0da4

Add callbacks

49adf99

CRImier reviewed Oct 22, 2018

View reviewed changes

derivmug and others added 22 commits October 22, 2018 20:59

Less magic numbers

667fb0e

Use BaseListBackgroundableUIElement

b07397d

Add generate_keymap

a79bd02

Add override_left

080efe0

Merge branch 'devel' of github.com:ZeroPhone/ZPUI into grid-menu

50b5b6b

Better movement

f9c8691

Comments

3d0ac63

Remove debugging print statement

e200465

Fix callback

7905956

Use Menu instead of BaseListBackgroundableUIElement

b8aa489

Add kind of View (not sure)

b9e5718

For now, renaming the "main menu" app to "grid menu test" app

218a8f5

Add downward scrolling logic

7e2bca2

Add scrolling down

9376d0e

Add entry_index to select_entry method of Menu

50e0595

Merge branch 'grid-menu' of github.com:ZeroPhone/ZPUI into grid-menu

2d08719

Test expand example app

d99f6ee

Fix bug where one would enter empty cells on scrolling

3424cd3

Moving the test app again

3a4fe5d

Moving hardware test app into a separate folder

6551bbe

Renamed ZP test app

75bfb8c

fixing Canvas.invert(), adding tests for it (only testing it doesn't …

79c4ef2

…throw an exception, for now)

CRImier added 5 commits November 6, 2018 15:06

Adding screen test functions to hardware test app

bd33855

Hardware test app now has a menu but also allows full test; started U…

4d397d1

…SB port testing function but haven't finished it yet

Merge branch 'devel' of github.com:ZeroPhone/ZPUI into grid-menu

a4c82e6

Intermediate changes - WIP (also, accidentally committed Hardware tes…

00604ec

…t app changes, no time to split)

Still WIP, but tests no longer fail

1fc74a3

CRImier added 18 commits November 27, 2018 19:20

Changing page_down and page_up to actually rely on move_up and move_d…

f6cd4c8

…own, making sure they don't refresh more than once per call

entry_height now doesn't need to be passed as an argument

732762d

Moving GridMenu display further into the GridView, changing it to be …

f747a56

…a view mixin. WIP - out-of-screen entry navigation doesn't yet work properly, might need to adjust first_displayed_entry better.

Forgot to return the keymap, fixing and improving (so that Menu keys …

84d118e

…also get added)

Adding ProHelper to helpers - with tests (but no docs yet), not yet r…

532fd4f

…emoving it out of pyavrdude because can't test it yet

ProHelper: use_terminal=True by default

2e3e37f

Merge branch 'devel' into grid-menu

55a0129

Refactoring grid_menu to depend on first_displayed_entry more, but is…

a3abf7b

…n't yet fully working

Merge branch 'devel' of github.com:ZeroPhone/ZPUI into grid-menu

61c9228

Avoid duplicating the mode variable too much

377e55e

Fixed RGB inversion in Canvas(ping @LouisPi , need to test)

f348f1b

Canvas full-screen inversion now a little bit safer

b77dab7

Improving the sandbox app a little (now the emulator window appears o…

e7559b7

…n load)

Merge branch 'devel' into grid-menu

91eabbf

GridMenu navigation working - now onto Entry objects

d0c001d

Fixed the link to emulator setup instructions

c5dc2a7

Adding Entry objects and support for them in most BaseListUIElement-b…

d78cfc8

…ased UI elements; adding tests for it

Adding DialogBox docs, fixing docs a little

c4021ba

CRImier merged commit c4021ba into devel Dec 23, 2018

CRImier deleted the grid-menu branch December 23, 2018 15:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Grid menu #123

Grid menu #123

derivmug commented Oct 22, 2018

CRImier left a comment

CRImier Oct 22, 2018

CRImier Oct 22, 2018

CRImier Oct 22, 2018

CRImier Oct 22, 2018

CRImier Oct 22, 2018

CRImier Oct 22, 2018

CRImier commented Oct 22, 2018

codecov bot commented Oct 22, 2018 •

edited

CRImier commented Nov 27, 2018

CRImier commented Nov 27, 2018 •

edited

CRImier commented Dec 23, 2018

	if new_x >= 1 and new_x <= 3:
	if new_x >= 1 and new_x <= self.GRID_WIDTH:
	if new_y >= 1 and new_y <= self.GRID_HEIGHT:

Grid menu #123

Grid menu #123

Conversation

derivmug commented Oct 22, 2018

CRImier left a comment

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier Oct 22, 2018

Choose a reason for hiding this comment

CRImier commented Oct 22, 2018

codecov bot commented Oct 22, 2018 • edited

Codecov Report

CRImier commented Nov 27, 2018

CRImier commented Nov 27, 2018 • edited

CRImier commented Dec 23, 2018

codecov bot commented Oct 22, 2018 •

edited

CRImier commented Nov 27, 2018 •

edited