Restore TUI scrolling to old behavior

[hasufell]

@lsmor

As discussed: #850 (comment)

[lsmor]

I think this does not recovers completelly old behaviour since it jumps if the section below doesn't fit on the screen

[hasufell]

I think this does not recovers completelly old behaviour since it jumps if the section below doesn't fit on the screen

It doesn't jump here:

[hasufell]

Here's the trick: we compute r (the row-location relative to the current element) based on the focused element of the focused section (see below)

  -- compute the location to focus on within the active section
  (c, r) :: (Int, Int) = case sectionListSelectedElement ge of
                           Nothing -> (0, 0)
                           Just (selElIx, _) -> (0, selElIx)

And then use the smallest possible dimension to focus on: (1, 1).

This only works with visibleRegion, not with visible.

[lsmor]

Honestly, I'm quite puzzled that this works... hahaha afaiu (c,r) are relative to the inner section, so when we call Brick.visibleRegion (Location (c, r)) ... it sends a visibility request to the outter viewport (a viewport for the whole section-list) using the coordinates of the inner section... Any case, It works and it is very clear

[lsmor]

It doesn't jump here:

Probably somthing weird with the vscode terminal. It doesn't jump on gnome's. In case you wonder It was a little bit easier to do, since in the version at #850 the row-location was already calculated in the variable off. By using the trick of minimizing the display area (1,1) you can use visibleRegion to push a request visibility for the current row

renderSectionList render_elem section_focus (GenericSectionList focus elms sl_name) =
    Brick.Widget Brick.Greedy Brick.Greedy $ do
        c <- Brick.getContext
        let -- A section is focused if the whole thing is focused, and the inner list has focus
            section_is_focused l = section_focus && (Just (L.listName l) == F.focusGetCurrent focus)
            s_idx = fromMaybe 0 $ V.findIndex section_is_focused elms
            render_inner_list has_focus l = Brick.vLimit (length l) $ L.renderList (\b -> render_elem (b && has_focus)) has_focus l
            (widget, off) = 
                V.ifoldl' (\wacc i list ->
                                let has_focus_list = section_is_focused list
                                    (!acc_widget, !acc_off) = wacc
                                    new_widget = if i == 0 
                                                 then render_inner_list has_focus_list list
                                                 else hBorder <=> render_inner_list has_focus_list list
                                    new_off 
                                        | i < s_idx  = 1 + L.listItemHeight list * length list 
                                        | i == s_idx = 1 + L.listItemHeight list * fromMaybe 0 (L.listSelected list)
                                        | otherwise  = 0
                                in  (acc_widget <=> new_widget, acc_off + new_off)
                                )
                (Brick.emptyWidget, 0)
                elms
        Brick.render 
          $ Brick.viewport sl_name Brick.Vertical
          -- This is the only change needed. for some reason off isn't enoughm, but off-1 does the job
          $ Brick.visibleRegion (Brick.Location (0, off - 1)) (1,1) widget

[hasufell]

the row-location was already calculated in the variable off

That's the row-location across all sections afaiu, not just the currently active section. I think we don't need that calculation anymore, since we're not translating across all sections.

As such I find my version simpler, since the offset calculation is completely removed.

[hasufell]

@lsmor can you accept the repo acces invite so I can add you as reviewer?

[hasufell]

Am not sure if the whitespace commit stuff will cause issues for your other PR...

[runeksvendsen]

I'm not convinced it would make sense for me to spend time reading and understanding the code, so I just compiled and ran the TUI locally, and can confirm that it fixes the issue in question.

[hasufell]

I dropped the whitespace commit