tview icon indicating copy to clipboard operation
tview copied to clipboard

Issues with focus/blur when using flex layout

Open carpii opened this issue 1 year ago • 8 comments

I'm using a flex layout, containing two frames. Top frame contains a form, bottom frame contains a table. I'd like to handle focus/blur on both frames, but the docs suggest this is not possible when they are contained in a flex or grid.

So instead, I attach handlers to the form and table, but these never fire either. It's not clear whether this is a bug, or just a known issue.

It feels like quite a major limitation though, since all but the most trivial apps will require some sort of layout container


image


package main

import (
	"log"
	"os"

	"github.com/gdamore/tcell/v2"
	"github.com/rivo/tview"
)

func ui_debug(s string) {
	l := log.New(os.Stderr, "", 0)
	l.Println(s)
}

func ui_table_header_cell(table *tview.Table, x int, y int, align int, txt string) {
	table.SetCell(x, y, tview.NewTableCell(txt).SetTextColor(tcell.ColorWhite).SetBackgroundColor(tcell.ColorDodgerBlue).SetAlign(align).SetSelectab
le(false))
}

func main() {
	err := test_focus_bug()
	if err != nil {
		panic(err)
	}
}

func test_focus_bug() error {
	app := tview.NewApplication().EnableMouse(true)

	// create test form
	form := tview.NewForm().
		AddInputField("Test", "TEST INPUT 1", 0, nil, nil).
		AddInputField("Test", "TEST INPUT 2", 0, nil, nil)

	// create test table
	color := tcell.ColorWhite
	table := tview.NewTable().SetBorders(false).SetSelectable(true, false)
	ui_table_header_cell(table, 0, 0, tview.AlignLeft, "TEST TABLE")
	table.SetCell(1, 0, tview.NewTableCell("TEST ROW").SetTextColor(color).SetAlign(tview.AlignLeft))
	table.Select(1, 0).SetFixed(1, 0)

	// create top frame
	frame_top := tview.NewFrame(form)
	frame_top.SetBorder(true).SetTitleAlign(tview.AlignLeft).SetTitle(" Frame Top (form) ")

	// create bottom frame
	frame_bottom := tview.NewFrame(table)
	frame_bottom.SetBorder(true).SetTitleAlign(tview.AlignLeft).SetTitle(" Frame Bottom (table) ")

	// attempt to handle focus/blur events - none of these handlers ever fire
	frame_bottom.SetFocusFunc(func() {
		ui_debug("[FOCUS] frame BOTTOM")
	}).SetBlurFunc(func() {
		ui_debug("[BLUR] frame BOTTOM")
	})

	frame_top.SetFocusFunc(func() {
		ui_debug("[FOCUS] frame TOP")
	}).SetBlurFunc(func() {
		ui_debug("[BLUR] frame TOP")
	})

	form.SetFocusFunc(func() {
		ui_debug("[FOCUS] form TOP")
	}).SetBlurFunc(func() {
		ui_debug("[BLUR] form TOP")
	})

	table.SetFocusFunc(func() {
		ui_debug("[FOCUS] table BOTTOM")
	}).SetBlurFunc(func() {
		ui_debug("[BLUR] table BOTTOM")
	})

	// create flex container
	flex := tview.NewFlex().SetDirection(tview.FlexRow)
	flex.AddItem(frame_top, 0, 3, true)
	flex.AddItem(frame_bottom, 0, 1, false)
	if err := app.SetRoot(flex, true).Run(); err != nil {
		return err
	}

	return nil
}

carpii avatar Aug 25 '23 19:08 carpii

I responded to #869... maybe can help?

https://github.com/rivo/tview/issues/869#issuecomment-1694233740

digitallyserviced avatar Aug 26 '23 08:08 digitallyserviced

I'm following up on this issue but the code above cannot be run (no main and it also references functions that don't exist). Can you please post code that can be run?

rivo avatar Apr 04 '24 16:04 rivo

Hi,

thanks for looking into this. I've updated the above code so it will build debug output is sent to stderr...

go build test_ui.go
./test_ui 2>./debug.log

then tail -f debug.log in a sep shell

The issues Im having..

Using mouse to select top and bottom frames, only ever emits [FOCUS] table BOTTOM and [BLUR] table BOTTOM

There are never any FOCUS or BLUR events emitted for the top and bottom frames, nor for the form (in the top frame)

Since its able to emit events for the bottom TABLE, I expected it to do the same for the TOP FORM

Ideally I'd like to see events emitted for the frames themselves, and then perhaps the control contained within that frame which is getting the focus

I acknowledge mouse support in terminal is never ideal, and my app will primarily use keyboard to navigate. But even then if I am not getting the control or frame events, its making it quite difficult to develop without maintaining my own representation of the ui state

carpii avatar Apr 04 '24 18:04 carpii

Thank you. When I run your code, after clicking on the table first, then on the form, I get this in debug.log:

[BLUR] frame TOP
[BLUR] form TOP
[FOCUS] table BOTTOM
[BLUR] table BOTTOM

And I'm actually surprised that the "blur" event is invoked on the frame and the form. Mostly because the documentation says this:

SetBlurFunc sets a callback function which is invoked when this primitive loses focus. This does not apply to container primitives such as Flex or Grid.

Frame and Form are both also container primitives. A Frame contains another primitive. A Form contains multiple other primitives (input fields, checkboxes, buttons etc.). When you click on a flex item to direct the focus to it, the Flex component passes the click on to the item itself. Then the item, e.g. your Form, passes it on again to the form item at the click position, e.g. an input field. (If there is no item at that position, the last selected item is chosen.)

So your click, and therefore your focus/blur event, is only ever processed by a "leaf" in the layout tree and never by the nodes inbetween that leaf and the root node. (On a side note, when a border is drawn, the HasFocus() method is called which, for container primitives, will call its contained primitives — essentially traversing the tree.)

Long story short, the behaviour you're seeing is expected. I'm not sure how easy it would be to change this (and how it would affect existing applications). In any case, I would like to understand first what you're trying to achieve. Maybe you can explain a bit why your application needs this? There may be other ways to solve the issue you're having.

rivo avatar Apr 06 '24 12:04 rivo

So it seems that the "blur" events on the frame and the form are called during initialization. They briefly have focus when they are added to the layout and then lose it again when the other elements are added.

rivo avatar Apr 06 '24 12:04 rivo

Ok, thanks. I wanted to make sure its defined behavior I can rely on, rather than just a bug.

For use case, it will vary. I have lots of these multi frame screen layouts (sometimes with 3 frames, but mostly 2), and was looking for a generic way to detect when focus moves from one frame to another

From what I recall, in some cases I want to hit the db when the bottom frame recieves focus, or other times it will manipulate controls such as changing the bottom table from readonly to editable when its frame has focus).

There's no doubt a bunch of other use cases Ive since forgotten about, since development on this app has been stalled for a good while

I guess my plan now is to add a generic focus handler to every focusable control, peek which frame it belongs to, and if the 'focused frame' has changed, then simulate a frame focus event and invoke my screen-specific handler.

Unless you have any suggestions for a better way?

Cheers

carpii avatar Apr 06 '24 20:04 carpii

Well, since the same has been requested in #869, I might have to think about how to make this possible in container primitives as well. So far, the implementation is very simple: I keep a reference to the currently focused item. Then I can call Blur() on that item when it loses focus.

SetFocus() is also very simple: It just updates the reference.

But if I want to include all primitives that the newly (or previously) focused element is contained in, I have to search the entire layout tree for it. There is currently no upwards reference (i.e. a parent pointer) on elements and I prefer not to introduce that. (It would bring with it many other problems.)

It's probably not difficult to implement but it will introduce a performance penalty. I guess it's acceptable, though. I don't think anyone has millions of input fields in their application.

rivo avatar Apr 07 '24 12:04 rivo

There is currently no upwards reference (i.e. a parent pointer) on elements

Ah I remember now. I think that's why I didnt just go ahead and implement the plan I mentioned above (since its not possible to take a control and identify which container it belongs to)

Any improvements you can come up with would be very welcome, and I think would probably be useful for other apps where the UI is dynamically created based on external data.

carpii avatar Apr 08 '24 20:04 carpii