fyne icon indicating copy to clipboard operation
fyne copied to clipboard

NavigationContainer Widget

Open lherman-cs opened this issue 4 years ago • 17 comments

Is your feature request related to a problem? Please describe:

I want to organize my application to "pages" similar to how web pages usually work so that my application can be scalable.

Is it possible to construct a solution with the existing API?

No, the closest solution I could get was by using some flags to mark some widgets to be hidden/shown. This approach is hard to scale, it becomes more annoying when I want to navigate from one page to another.

Describe the solution you'd like to see:

I've created my own router which you can see the implementation here, https://github.com/pion/obs-wormhole/blob/master/router.go and an example how to use the API, https://github.com/pion/obs-wormhole/blob/master/main.go. The concept is very similar to react-router.

I'm more than happy to create my implementation as a library or as a part of fyne!! But, before I do that, I'd like to know if there's already an existing router widget (I don't want to reinvent the wheel) or an alternative approach that basically can do what I want, separate pages to make applications more scalable.

By the way, this project is pretty awesome! Great job authors and maintainers!

lherman-cs avatar Jul 04 '20 23:07 lherman-cs

Sounds really cool. Could such a widget be called “NavigationContainer’? If so I’d imagined something that provides user navigation as well. I think your API is overall quite “React” instead of “Go/Fyne” but the concept is something I could see is including

andydotxyz avatar Jul 06 '20 21:07 andydotxyz

NavigationContainer sounds good to me. Yes, it'll provide navigation as well, you can see an example here, https://github.com/pion/obs-wormhole/blob/master/home.go#L37-L39.

Yup, I was inspired by how they define routing declaratively. I'm down with changing the API to be more "Go/Fyne", but I'm not sure exactly what you mean or how to make it more "Go".

lherman-cs avatar Jul 06 '20 21:07 lherman-cs

I can't relly see the navigation in that - do you have a screenshot so we can more easily tell how it would work? If you'd like to consider a core widget using this perhaps propose here, or in a wiki page how the API might be adapted to feel more fitting with our other widgets?

andydotxyz avatar Jul 07 '20 19:07 andydotxyz

Design

This proposal is about adding a new widget called NavigationContainer. The widget will be used as a page view and navigation. The design is inspired by declarative routing design from react-router and has been modified to be suitable for "Go/Fyne".

New interfaces

These interfaces should go under https://pkg.go.dev/fyne.io/fyne?tab=doc

Navigator

This interface will be used to hide implementation details and meant to be passed around between pages so that they can use the interface to navigate.

type Navigator interface {
	Push(path string, ctx interface{}) error
	Pop() error
	Reset()
}

Page

This interface will be an optional interface that will be used for NavigationItem. The idea of this interface is to provide each page hooks to the lifecycle, it's extremely useful to manage resources, e.g. close socket connections. Users should be able to provide only fyne.Widget not Page, this will provide flexibility to the users that they don't have to implement BeforeDestroy, this is especially useful for small apps.

type Page interface {
	fyne.Widget
	BeforeDestroy()
}

New Widget

These should go under https://pkg.go.dev/fyne.io/[email protected]/widget?tab=doc

NavigationHandler

This provides a type how to handle a path, essentially a page builder. This is similar to how fyne handles custom data, e.g. ButtonAlign, ButtonStyle, etc.

// notice that we only require fyne.Widget instead of Page.
type NavigationHandler func(navigator Navigator, ctx interface{}) (fyne.Widget, error)

NavigationItem

This struct provides a definition of a path and how to handle it. This is going to be used for the navigation itself, each time we push a path, the handler will be called to create a page.

Similar to fyne widgets, the naming is suffixed with Item and its properties are exposed. Also, it provides a constructor to make the syntax easier for users.

type NavigationItem struct {
	Path    string
	Handler NavigationHandler
}

func NewNavigationItem(path string, handle func(Navigator, interface{}) (fyne.Widget, error)) *NavigationItem {
}

NavigationContainer

This struct is going to be the actual widget. Like any other fyne widgets, this widget also has a constructor and follow the same pattern, if there are many items, variadic argument is used.

This widget also implements Navigator which will give users an ability to use navigator across pages through NavigationHandler.

Not only that, NavigationContainer allows setting a hook, e.g. BeforeEnter. BeforeEnter is a hook function that is going to be called everytime users navigate to another page. The function might return an error, in which case Push will also return an error and the navigation will be canceled.

type NavigationContainer struct {
	widget.BaseWidget
	BeforeEnter  func(to string) error
	routes       map[string]NavigationHandler
	history      []fyne.Widget
	currentPage  fyne.Widget
}

func NewNavigationContainer(initialPath string, items ...*NavigationItem) (*NavigationContainer, error) {}
func (navigation *NavigationContainer) Push(path string, ctx interface{}) error {}
func (navigation *NavigationContainer) Pop() error {}
func (navigation *NavigationContainer) Reset() {}

Usage

For full implementation: https://github.com/lherman-cs/fyne-navigator/

package main

import (
	"fmt"

	"fyne.io/fyne"
	"fyne.io/fyne/app"
	"fyne.io/fyne/widget"
)

type Page2 struct {
	fyne.Widget
}

type Page2Context struct {
	Label string
}

func NewPage2(navigator Navigator, ctx Page2Context) fyne.Widget {
	var content *widget.Box
	label := widget.NewLabel(ctx.Label)
	button := widget.NewButton("Pop", func() {
		err := navigator.Pop()
		if err != nil {
			panic(err)
		}
	})
	content = widget.NewVBox(
		label,
		button,
	)

	return &Page2{
		Widget: content,
	}
}

func (page *Page2) BeforeDestroy() {
	fmt.Println("Page2: BeforeDestroy")
}

func main() {
	var err error

	a := app.New()
	w := a.NewWindow("Hello")

	page1 := NewNavigationItem("/page1", func(navigator Navigator, ctx interface{}) (fyne.Widget, error) {
		fmt.Println("Pushing /page1")
		return widget.NewVBox(
			widget.NewButton("Push", func() {
				navigator.Push("/page2", Page2Context{"From Page 1"})
			}),
		), nil
	})

	page2 := NewNavigationItem("/page2", func(navigator Navigator, ctx interface{}) (fyne.Widget, error) {
		fmt.Println("Pushing /page2")
		return NewPage2(navigator, ctx.(Page2Context)), nil
	})

	router, err := NewNavigationContainer("/page1", page1, page2)
	if err != nil {
		panic(err)
	}

	router.BeforeEnter = func(to string) error {
		fmt.Println(to)
		return nil
	}
	w.SetContent(router)
	w.ShowAndRun()
}

lherman-cs avatar Jul 08 '20 02:07 lherman-cs

What do you think about the design @andydotxyz ?

lherman-cs avatar Jul 11 '20 00:07 lherman-cs

Some good stuff here, thanks. Not sure that the string (path?) parameters add much to the design?

Where you have used fyne.Widget it should probably be fyne.CanvasObject. From the overview it seems like this could be achieved without Navigation interface?

BeforeEnter and therefore Push returning error is rather confusing - what is the use-case here?

andydotxyz avatar Jul 11 '20 20:07 andydotxyz

It's a path, I was inspired by URLs, I think it's a very elegant concept to represent different pages. Of course, there's not restriction to what format that you should follow, but this can be easily extended in the future or formalized in the future if we want to.

Where you have used fyne.Widget it should probably be fyne.CanvasObject.

That's definitely a good idea! Since it's a lower level interface, we can provide more structs/interfaces.

From the overview it seems like this could be achieved without Navigation interface?

It is an attempt to protect NavigationContainer from being modified. But, it seems that hiding the properties is enough. So, I agree that it should be a struct.

BeforeEnter and therefore Push returning error is rather confusing - what is the use-case here?

There are many use cases for having a global hook for BeforeEnter, e.g. authentication, logging, metrics, etc. These use cases can definitely be implemented before calling push or at the route registration. But, these approaches will force the users to put the hook logic in an awkward place, within a widget event callback or page builder function, and the logic can't be easily reused between paths without duplicating calls.

The error is being used to mark either the transition to the new path is successful. Imagine if we want to guard a page from unauthenticated users. We want to return an error for unauthenticated users so that the navigation fails, and forces users to stay on the current page. Then, the push caller will get notified by this error by forwarding that error.

lherman-cs avatar Jul 14 '20 02:07 lherman-cs

I see now that you're designing this as the the content were web page(-like) which does indeed things like authentication middleware. However, this is all code-created content - if a "page" needs intercepts we know this and can display the login page instead.

From the design I could not understand what the page string would be used for, after being registered - was there to be some sort of lookup or reference mechanism that would make use of this? (apologies if I overlooked the usage).

If we are to add some navigation UI then I suppose each time something is pushed to the stack it might need {title string, content CanvasObject}. The user navigation would just be Back to pop current page and forward navigation would always be within code I think (buttons etc). I'm always keen that we make the simplest possible solution - apologies if I have over-simplified.

andydotxyz avatar Jul 14 '20 20:07 andydotxyz

I see now that you're designing this as the the content were web page(-like) which does indeed things like authentication middleware. However, this is all code-created content - if a "page" needs intercepts we know this and can display the login page instead.

Do you mind to elaborate? Are you saying that the same logic can be done inside of individual page instead of global?

From the design I could not understand what the page string would be used for, after being registered - was there to be some sort of lookup or reference mechanism that would make use of this? (apologies if I overlooked the usage).

This was used for building the lookup table at the beginning and for a quick lookup for push. There are some benefits of doing this:

  1. It's easier to manage the dependency graph between pages since you can simply use strings or you can put all the paths as const in 1 package and let all pages to depend on a single package.
  2. Since it's just a string, people can do a lot of interesting stuff with BeforeEnter hook. Logging or metrics would be very easy. You can also think of path as a unique logical label per page.

If we are to add some navigation UI then I suppose each time something is pushed to the stack it might need {title string, content CanvasObject}. The user navigation would just be Back to pop current page and forward navigation would always be within code I think (buttons etc).

You only need to give the path, there's no need to give the content since the navigator already has all of the references to how to build the content.

I'm always keen that we make the simplest possible solution - apologies if I have over-simplified.

No worries! I love simplicity also, that's why I'm using Go lol 😃. I just want to make sure that we're on the same page.

lherman-cs avatar Jul 15 '20 00:07 lherman-cs

Do you mind to elaborate? Are you saying that the same logic can be done inside of individual page instead of global?

Well, if we are constructing the page in code, on demand, then you can very easily determine if authentication should be presented instead of the expected page.

This was used for building the lookup table at the beginning and for a quick lookup for push

I don't quite follow why a string representation is easier than keeping track of an object reference?

i.e. we can type check myPage := newAccountScreen() and push myPage into nagivation any time. But if you PushPage(/account`) there are a whole heap of errors we need to handle that cannot be known until runtime (typos, page not found etc). If you want to put logging into your components then you can also take the code approach - as you will be pushing items into the stack using code it's trivial to log the action directly rather than callbacks on actions?

navigator already has all of the references to how to build the content.

I guess I don't quite see why delegating "how to build my screen" to a navigator is clearer than just building the content directly and pushing into the navigation stack?

When I referred to "navigation UI" I was not meaning the code to navigate, I was thinking more like: https://i.stack.imgur.com/CNmRh.png - we could generate the user controls to show context and going back to previous...

andydotxyz avatar Jul 15 '20 12:07 andydotxyz

I like compile time checks, and don't mind getting rid of these strings and build the page on demand. But, here are some concerns that I have:

  1. How to handle a circular dependency routing? For example, page1 pushes page2 and page2 pushes page1 assuming page1 and page2 live in 2 different packages.
  2. How are we going to keep it DRY for logging/metrics for the global hook, BeforeEnter? It's indeed trivial, but it's going to be repetitive to call the same logging function. When logging or metrics are not so basic, I think people would normally abstract these away in separate structs/interfaces. Should users need to come up with their own solution to pass around the struct/interface? I would think that people will use some sort of dependency injection pattern?

lherman-cs avatar Jul 16 '20 02:07 lherman-cs

Thanks, these are interesting questions.

  1. This will depend on how you are pushing pages. One solution would be that your application stitches together the components - i.e. main can use packages package1 and package2 to coordinate pushing the pages at the right time. Alternatively you could advertise an Application interface that exposes, for example Page1() fyne.CanvasObject function so that pacakge 2 could get the current app and access Page1. That said, for the most part I don't think it's good practice to design a circular user path through an app...
  2. When it comes to DRY you need to consider the whole toolkit not just a single component. Logging that something has been displayed applies to TabContainer, Container and maybe even ScrollContainer as well - so a generic approach to such callbacks would need to work in these situations as well. In TabContainer we have an "OnChanged" so that code can understand what a user action resulted in - but in the navigation when we are pushing and popping in code this is not required. I guess I don't see a single function call such as LogTransition("reason") as violating DRY as the line has a purpose and communicates it minimally by invoking a single log helper.

Oh, the alternative approach I suggested does not require that pages are built on demand - if you want you can still construct the whole app UI on load, then add or remove components using variable references where you previously had strings.

andydotxyz avatar Jul 16 '20 10:07 andydotxyz

Tidying up issues and I found this. I still think it's a good idea - and without the string routing the new design is quite familiar to Fyne devs. I think it could work without NavigationHandler, plus NavigationItem and Page could probably be combined. Now we have a container package I think this would make sense as container.NewNavigation

andydotxyz avatar Nov 17 '20 18:11 andydotxyz

I have built a AppBars and NavigationDrawer element which would both greatly benefit from the ability for applications to be "Navigatable" in the sense that there is a context memory of "back" and "up" when it comes to navigation from one pane to another (such as sub-content.)

You can see the existing work here: https://github.com/AlbinoGeek/sc2-rsu/tree/main/fynex

I plan to merge these into fyne-x after cleaning up the renderer and a few other fixes.


My next step along this (after cleanup) was very literally to implement navigation.

AlbinoGeek avatar Dec 16 '20 01:12 AlbinoGeek

Did anything ever come of this? A fyne-endorsed example of how to navigate effectively, efficiently, and idiomatically would be helpful for adoption’s sake, if nothing else.

atljoseph avatar Mar 28 '24 12:03 atljoseph

We have not progressed yet - the discussion above is all we have. I also looked at #3044 but that was more than navigation.

andydotxyz avatar Apr 01 '24 17:04 andydotxyz

If it's helpful for anyone, I have a custom navigation solution built for Supersonic, it's in the ui/browsing package, with BrowsingPane being the top-level component. Note that all the "pages" also make use of ui/util/widgetpool.go to re-use common widgets across different pages for performance reasons.

dweymouth avatar May 08 '24 15:05 dweymouth