Fit message on screen when setting focus with "move"
Is your feature request related to a problem? Please describe.
The "move" command sets the focus on the title line of a message in thread mode. If this line is already visible at the bottom of the screen, but not the message itself, the view isn't scrolling by itself to show the message.
Describe the solution you'd like
The view should be scrolled to fit the message, or at least its beginning, in the viewport.
Describe alternatives you've considered
A dirty workaround that doesn't work for the last message in a thread is to rebind a key to "move next; move previous".
Additional context
/
Agreed, but I don't know out of the top of my head how do do this. The thread view depends on urwids ListBox which, afaik, scrolls to the "right position" for you..
Bur feel free to dig in and send a PR :)
Having just started using alot and wanting the same feature, I thought I'd have a look, and I think I have a working solution - though it requires playing with urwidtrees rather than sticking in the alot codebase.
The 'fix' is to change the following:
https://github.com/pazz/urwidtrees/blob/d1fa38ce4f37db00bdfc574b856023b5db4c7ead/urwidtrees/widgets.py#L122-L123
to be:
def set_focus(self, pos):
self._outer_list.set_focus(pos)
return self._outer_list.set_focus_valign('top')
ListBox.set_focus_valign is documented here: http://urwid.org/reference/widget.html#urwid.ListBox.set_focus_valign
Since this probably wants to be optional, I'm guessing one approach would be to make self.set_focus take an optional valign param, and only call set_focus_valign if it is passed in? Though I don't know how many other points in the code make this call, and whether there should instead be a separate valign version of the method?
Happy to make a PR if this is the right approach, but also happy for someone more familiar with the codebase to decide on how best to implement this :smile:
This sounds like a good plan. I'm very open for PRs, both here and for urwidtrees. In fact the hope was for urwidtrees to be merged into urwid at some point but no-one could be bothered to take over so far..
In any case, moving a message content into focus sounds like a good default. But we should make sure that the move commands are unaffected in case the messages are folded (the body is not visible).
Quoting Matthew Richardson (2020-04-13 21:19:20)
Having just started using alot and wanting the same feature, I thought I'd have a look, and I think I have a working solution - though it requires playing with urwidtrees rather than sticking in the alot codebase.
The 'fix' is to change the following:
https://github.com/pazz/urwidtrees/blob/ d1fa38ce4f37db00bdfc574b856023b5db4c7ead/urwidtrees/widgets.py#L122-L123
to be:
def set_focus(self, pos): self._outer_list.set_focus(pos) return self._outer_list.set_focus_valign('top')
ListBox.set_focus_valign is documented here: http://urwid.org/reference/ widget.html#urwid.ListBox.set_focus_valign
Since this probably wants to be optional, I'm guessing the best approach would be to make self.set_focus take an optional valign param, and only call set_focusvalignif it is passed in? Though I don't know how many levels of indirection would be needed betweenalot` and this call...
Happy to make a PR if this is the right approach, but also happy for someone more familiar with the codebase to decide on how best to implement this 😄
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.*
Sorry for the delay on this.
I'll merge your PR in urwidtrees just now. Next, we would need a short patch for alot that makes use of this new feature.
I am not sure how many users would actually not want to automatically make newly focussed mails visible, so I believe we could hard-code this behaviour. Alternatively, if you insist, there could be a config setting.
Either way, it will be necessary to update alots dependency on an urwid version that includes the feature. I can make a urwidtrees release for that in order to facilitatate upcoming alot releases..
#1491 does not seem to fix this behaviour for me.
Did you meant #1528 (PR)? It doesn't seem to be enough to fix this issue for me either. Please re-open.
yes sorry, I meant #1528.
Apologies - the rest of the fix (I hope!) is now in #1535