issadarkthing / gomu

golang TUI music player
GNU General Public License v2.0
159 stars 15 forks source link

Focus control #36

Closed tramhao closed 3 years ago

tramhao commented 3 years ago

I added a new PR for tag editing and lyric previewing. This is a flex contains a bunch of other controls. I met a problem: When I use popup message boxes to show the messages, the focus can fallback to flex, but will always focus on the first control in the flex. It's not intuitive. How to avoid focus switching? As the message boxes don't need focus. Thanks in advance.

issadarkthing commented 3 years ago

Maybe we should refactor to not switch focus on message popups at all

tramhao commented 3 years ago

I'm not so familair with tview yet, but what's your design purpose to add info popups into the pages? Thanks.

issadarkthing commented 3 years ago

It needs to be inside the pages in order to make it appear in front of other widgets.

tramhao commented 3 years ago

ok, thanks for the info. That's something I didn't get from the beginning.

tramhao commented 3 years ago

I tried to comment out below code:

    resetFocus := func() {
        // timed popup shouldn't get focused
        // this here check if another popup exists and focus that instead of panel
        // if none continue focus panel
        topPopup := gomu.popups.peekTop()
        if topPopup == nil {
            gomu.app.SetFocus(gomu.prevPanel.(tview.Primitive))
        } else {
            gomu.app.SetFocus(topPopup)
        }
    }

    resetFocus()

    go func() {
        time.Sleep(timeout)
        gomu.app.QueueUpdateDraw(func() {
            gomu.pages.RemovePage(popupID)
            resetFocus()
        })
    }()

But is not working as expected.

issadarkthing commented 3 years ago
    widget := gomu.app.GetFocus()
    gomu.pages.AddPage(popupID, topRight(box, width, height), true, true)
    gomu.app.SetFocus(widget)

    go func() {
        time.Sleep(timeout)
        gomu.app.QueueUpdateDraw(func() {
            widget := gomu.app.GetFocus()
            gomu.pages.RemovePage(popupID)
            gomu.app.SetFocus(widget)
        })
    }()

This works on preventing the app to switch focus on popups but it somehow messes up the tag editor focus.

tramhao commented 3 years ago

The problem of tview.flex is, it doesn't have focus, but will always send focus to 1st control contained in it. It's quite difficult to handle the focus of flex. Not only popups, but also selection list will affect the focus of tageditor.

tramhao commented 3 years ago

Fix it in https://github.com/issadarkthing/gomu/pull/35/commits/67def0a86de1eedad8b4a9c0af3fe2e3bb269dc4