Mode/Widget Drawing Refactor Idea

[romanofski]

Quick summary from our discussion (I thought better create an issue to discuss progress with this idea). Problems:

• UI inconsistent for composing e-mail #139
• "Focusable" is misleading terminology #141
• mode proliferation (e.g. for each focused widget a new mode ManageThreadTags or ManageMailTags)

Idea:

• use a proof of concept app
• there is a global focus ring
• app state now contains current view (list of widgets)
• mode transitions manipulates this list. changing modes may replace the list entirely, or just parts of it (or make no changes?) For example, if in BrowseThreads and the "edit search" action runs, the edit search widget will be INSERTED or REPLACE the current edit search widget, and the thread list is preserved
• on mode transition, the relevant keybindings for the new widget(s) being inserted into (or replacing) the current view widgets (if any) are looked up in the config. The corresponding global focus ring is updated accordingly.
• the app state contains a global Map Name [Keybinding () (...)]. The keybindings for the new widgets are inserted into the map (the ctx phantom type being coerced to ()).
• when handling key events, the Name of the currently focused widget is looked up. Then the keybindings for the currently focused widget are looked up from the global key map (see previous point). Then the matching action (if any) is performed.

[romanofski]

Btw. we might need to find something else for Keybinding () (...), since the data type is:

data Keybinding (ctx :: Mode) = -- etc

and the compiler won't accept () since it has kind * ...

[romanofski]

Also the drawing function actually produces a list of widgets [Widget n] which are being rendered, which we can't really keep in the Application state. When we create the initial state, nothing has been rendered. But what we could do and that's currently my angle of approach is to use a list of (resource) names as a current view.

[frasertweedale]

Sounds fine to me.

[romanofski]

For the kind problem and stashing Keybindings in a Map, I've used an intermediary InternalKeybinding data type for now. Not sure if we need to settle on this, but at least it keeps me going for now. I guess we can always focus on that part if the overall concept works out. A function would convert the config keybindings to internal keybindings if push comes to shove.

I have a basic application setup now and will proceed of trying to manipulate the current view with the keybindings. The view is currently a simple list of resource names. I'm not really settled yet if mode transitions replace the entire list of names or if they just add/remove. We'll see...

I first had the idea of simply using the focus ring for rendering, but it has no Traversable or Functor instances .. so guess I'll have to keep two lists around for now.

One other idea I might try out is to look into GADTs in order to maybe construct a drawing data type so that I can say something like: this drawing function paints this widget (e.g. list of mails) for this mode (or view).

[frasertweedale]

Maybe we can use PolyKinds somehow. I'll have a play around soon...

[romanofski]

I've pushed my proof of concept here: https://github.com/romanofski/pidgets. What works is the dynamically replacing of widgets and it's rendering - yay. I can even get rid of the global mode. Points for thought:

• I need two lists/vectors to hold the widget names to render: one for the focus ring and one which constitutes which widgets I currently render. That means if I replace widgets in the current view, I'll need to do the same in the focus ring
• The keybindings probably need to abstract over the name instead of the mode.
• I have the feeling we want two different actions: one which only change focus and one which change/add/replace the widgets in the current view.
• If I have one drawing function for a widget, lets take the status bar, I wonder what a good way is to establish some kind of context. Hm.. just thinking out loud, maybe the context is the currently focused widget and the status bar could use some statusbar drawing function which renders specific things for the currently focused widget.

[romanofski]

Pushed another version and added more documentation onto functions. I feel this is the way to go. Way more flexible and we could still achieve the same effect as to what we have now. Furthermore, I think that we can build upon what we already have (I'm careful, since I haven't tried yet).

Unless you have anything I might just go for it and try it out in a branch.

[frasertweedale]

@romanofski nice work so far mate.

The next thing we need in the POC is the proper keybinding configuration and a way to enforce configuration sanity. We also want to avoid repeating keybinding definitions e.g. for various list widgets, we want DRYness; a way to define default "list widget" or "text input" keybindings etc.

I can also see some other way to improve the implementation. Will fork and play around tomorrow perhaps (or soon).

[romanofski]

Agreed. I haven't touched any possible changes to the keybindings, but kept the as simple as possible to test the idea of a more flexible drawing code. What I have left out so far is the mail view, which could have a resource name, but is a bit unlike any other widget in use so far (mostly the UI consists of lists and editors).

[romanofski]

Just in case to avoid any collision, if I have time I'll twiddle more with the drawing code.

[romanofski]

I was thinking about this:

a way to define default "list widget" or "text input" keybindings etc.

today for a while. We have the configuration with keybindings, then want to stash them in a Map Name Keybinding to look up the keybinding for the currently focused widget.

At this point, the Map would hold a specific keybinding and the default would be the keybinding we'd fall back defined by Brick itself. Thinking "aloud", maybe we need some kind of layered approach? Keep general keybindings which fit all list like widgets for example in a separate map, while if that lookup fails we try to look up the per-name keybindings and if that fails we go to brick?

[romanofski]

Dear @frasertweedale, have you had time to look into the keybindings? I'd be keen to look into it myself, but I can not remember nor reproduce how you pulled off the stunt with the -XPolyKinds. I've studied the Giving Haskell a Promotion paper for a bit, yet am not sure how you'd go to a PolyKinded data type, because () seems to have kind *. My question basically being is, that the kind language either allows you to specify a specific kind (like now data Keybinding (a :: Name) = --etc) or all kinds are allowed with *. How do you get the mix with: I only want to allow Name and ()? Do you use two type variables?

[romanofski]

I've continued with the keybindings. I split the Action constructor apart to have one designated for the widget and one is defined for everything:

data Action ctx a where
  AppAction :: String -> (AppState -> EventM Name a) -> Action ctx a
  Action :: String -> (Lens' AppState ctx -> AppState -> EventM Name a) -> Lens' AppState ctx -> Action ctx a

First problem I realised with multiple constructors: our chain function now needs 2^2 pattern matches. If I add another constructor, that becomes not very maintainable. Is there a better way to go about this?

I also threw out the keybinding map, just to realise that we actually want it. Trying to coerce the Action is a bit odd, since I would coerce the Action's lens to something like:

Lens' () ()

and it doesn't make much sense to me. I actually had to write it out as:

asFoo :: (Applicative f, Functor f) => (() -> f()) -> () -> f ()
asFoo f x = pure x

It checks out, but I'm worried once I've coerced the type and chucked the Action in the map, it'll be useless.

Anyway, I pushed my latest version to: https://github.com/romanofski/pidgets/tree/feature/use_lens_as_context_for_action in case you want to have a look.

[frasertweedale]

@romanofski I'll try and have a look in the next day or so.

[romanofski]

Argl... it happened again and forgot to actually sent my comment. Anyway, I'll post it now for completeness.

I've continued with the keybindings. I split the Action constructor apart to have one designated for the widget and one is defined for everything:

data Action ctx a where
  AppAction :: String -> (AppState -> EventM Name a) -> Action ctx a
  Action :: String -> (Lens' AppState ctx -> AppState -> EventM Name a) -> Lens' AppState ctx -> Action ctx a

First problem I realised with multiple constructors: our chain function now needs 2^2 pattern matches. If I add another constructor, that becomes not very maintainable. Is there a better way to go about this?

I also threw out the keybinding map, just to realise that we actually want it. Trying to coerce the Action is a bit odd, since I would coerce the Action's lens to something like:

Lens' () ()

and it doesn't make much sense to me. I actually had to write it out as:

asFoo :: (Applicative f, Functor f) => (() -> f()) -> () -> f ()
asFoo f x = pure x

It checks out, but I'm worried once I've coerced the type and chucked the Action in the map, it'll be useless.

Anyway, I pushed my latest version to: https://github.com/romanofski/pidgets/tree/feature/use_lens_as_context_for_action in case you want to have a look.

[romanofski]

I actually spend time to try to implement the current idea, but it's to undefined. It's easy to shuffle a few widgets around, but I don't think I could add this to the entire application. Instead of a global focusring, I tried a different approach. I don't know if the naming is right, but I thought of modeling each "page" as a view identified by a name:

data ViewName
    = Threads
    | Mails
    | ComposeView
    deriving (Eq,Ord)

data View = View
    { _vFocus :: Brick.FocusRing Name
    , _vWidgets :: [Name]
    }

data ViewSettings = ViewSettings
    { _vsViews :: Map.Map ViewName View
    , _vsFocusedView :: Brick.FocusRing ViewName
    }

Each view also holds his own focus ring and all the defined views are then stored on the application state. Changing views is a matter of moving the focused view focus ring around. I then have a define state of what a view would be rendered as, but still have the flexibility of adjusting/replacing widgets in the view if I so wanted. Furthermore - if ever - there is a possibility to "overlay" previous or other views (thinking of the mail editor here and jumping back to the index).

I'm still fiddling with the demo app, but it feels much easier to handle. Any thoughts? Maybe better names?

[romanofski]

I'm slowly getting to a point where I'm finished with the transition. It gives a little gain in comparison to master (re-use of widgets, more flexible rendering, additional ability to go back to the previous view etc). Overall I kept the UI as it is. I can see that the current implementation is quite immature (I've lost richer layouting e.g. the mail view takes up now 50% list of mails vs 50% showing the mail itself). What are your thoughts: merge once all acceptance tests pass, optimize afterwards or implement improvements now?

Current state is pushed to branch refactor/flexible_ui.

[frasertweedale]

@romanofski IMO merge as soon as the change is usable and meets the flexibility goals. Improve the actual layouts afterwards.