keyboard controllable popup menu

[toaster]

Description:

This PR implements keyboard control for pop-up menus.
For this it introduces a simple canvas object wrapper which might be elaborated into a solution for #709.

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.

[andydotxyz]

I can’t figure out why the new type was needed, it looks like a huge change for something that should have been less far-reaching.

1. can you explain by it was not possible to add the keyboard functions to the pop up menu which led to needing an additional object internally to wrap and receive keyboard events?
2. if we do need to add a wrapped object it should not require a whole new type - a helper that returns a Widget instance looks like it would offer the same functionality?

But as I said above these questions probably stem from me not understanding why the wrapped objects were required.

[toaster]

1. Because the PopUpMenu itself is not part of the rendered content. It only wraps the overlay container and the overlay’s content (the embedded Menu). This might or might not be refactorable into a better architecture when Inconsistent size/position handling for overlays #707 is solved.

2. The first proof-of-concept implementation actually implemented a widget instead of the CanvasObjectWrapper but that includes a lot of methods that are useless for the use-case. A widget is too heavy-weight for this, IMO.

[andydotxyz]

OK, 1. is a problem - I did not fully realise this was happening previously sorry. We should not have widgets that do not behave as widgets. We support extending widgets to do exactly what I had imagined would be possible. I think this is the source of complexity for the current ticket and I would not support adding another type to work around the complexity of the PopUpMenu type, sorry.

The 2. point is fair ehough, but should be moot if we solve 1..

[andydotxyz]

I don't think that the PopUpMenu not being visible is a requirement now that I look further, it looks more like it's constructor just does not create the overlay in a way that makese use of it's extending the Menu widget. I was able to apply the following changes and handle keyboard actions directly in PopUpMenu code:

diff --git a/widget/popup_menu.go b/widget/popup_menu.go
index 83345530..902db094 100644
--- a/widget/popup_menu.go
+++ b/widget/popup_menu.go
@@ -19,7 +19,7 @@ func NewPopUpMenu(menu *fyne.Menu, c fyne.Canvas) *PopUpMenu {
        p := &PopUpMenu{Menu: NewMenu(menu), canvas: c}
        p.Menu.Resize(p.Menu.MinSize())
        p.Menu.customSized = true
-       o := widget.NewOverlayContainer(p.Menu, c, p.Dismiss)
+       o := widget.NewOverlayContainer(p, c, p.Dismiss)
        o.Resize(o.MinSize())
        p.overlay = o
        p.OnDismiss = func() {
@@ -28,6 +28,24 @@ func NewPopUpMenu(menu *fyne.Menu, c fyne.Canvas) *PopUpMenu {
        return p
 }
 
+func (p *PopUpMenu) TypedKey(e *fyne.KeyEvent) {
+       switch e.Name {
+       case fyne.KeyEscape:
+               p.Hide()
+       case fyne.KeyDown:
+               p.ActivateNext()
+       case fyne.KeyUp:
+               p.ActivatePrevious()
+       }
+}
+
+func (p *PopUpMenu) TypedRune(rune) {
+}
+
+func (p *PopUpMenu) FocusGained() {}
+
+func (p *PopUpMenu) FocusLost() {}
+
 // ShowPopUpMenuAtPosition creates a PopUp menu populated with items from the passed menu structure.
 // It will automatically be positioned at the provided location and shown as an overlay on the specified canvas.
 func ShowPopUpMenuAtPosition(menu *fyne.Menu, c fyne.Canvas, pos fyne.Position) {
@@ -35,13 +53,6 @@ func ShowPopUpMenuAtPosition(menu *fyne.Menu, c fyne.Canvas, pos fyne.Position)
        m.ShowAtPosition(pos)
 }
 
-// CreateRenderer returns a new renderer for the pop-up menu.
-//
-// Implements: fyne.Widget
-func (p *PopUpMenu) CreateRenderer() fyne.WidgetRenderer {
-       return p.overlay.CreateRenderer()
-}
-
 // Hide hides the pop-up menu.
 //
 // Implements: fyne.Widget
@@ -72,6 +83,7 @@ func (p *PopUpMenu) Resize(size fyne.Size) {
 func (p *PopUpMenu) Show() {
        p.overlay.Show()
        p.Menu.Show()
+       p.canvas.Focus(p)
 }
 
 // ShowAtPosition shows the pop-up menu at the specified position.

[toaster]

This will be addressed by a new PR based on the current develop branch.