Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mail view: moving up/down at top/bottom of list should not "re-open" mail #281

Open
frasertweedale opened this issue Apr 3, 2019 · 7 comments
Labels
Milestone

Comments

@frasertweedale
Copy link
Member

Is your feature request related to a problem? Please describe.
When you are already at the first/last email in a thread, and you "go to previous/next mail",
you "move" to the same email, and lose your place in the mail.

Describe the solution you'd like
We should detect that the target email is currently-open email, and NOT "re-open" it.

For UX bonus points, maybe we display (temporarily) an "already at first/last mail" message, like mutt does.

@romanofski
Copy link
Member

Do you expect to go to the next mail in the thread? I'm currently not sure what you mean.

@frasertweedale
Copy link
Member Author

@romanofski I mean, when you're at the top and move UP, or when you're at the end and move DOWN, you should not lose your place (i.e. how far down it you've scrolled) in the currently-open mail. Does this clear it up?

@romanofski
Copy link
Member

To reproduce:

  • Open a mail in which the content extends beyond the view port and is either the first or the last mail.
  • Scroll down
  • Press next mail again and mail content starts at the top again

@romanofski romanofski added this to the 1.0 milestone Apr 3, 2019
@romanofski romanofski added the bug label Apr 3, 2019
@romanofski
Copy link
Member

I had a quick look. The current implementation:

  1. resets the scroll status
  2. parses the email by using the the e-mail filepath
  3. updates the AppState with the newly created MIMEMessage

There are a few options:

  1. We parse the message into a MIMEMessage and then do an equality check. Only update the scroll state if the messages are not equal. Downside: incur the cost of actually parsing the message every time
  2. We keep the NotmuchMail id of the current open mail somewhere in the AppState and do the equality check here. Upside: No additional parsing costs, but downside of the need to update another field.
  3. Any chance to identify a NotmuchMail with a MIMEMessage we could mitigate the downsides of 1) and 2)

@frasertweedale
Copy link
Member Author

@romanofski surely we can just perform the brick List move command and compare the index of previous and next list, or something along those lines...

@romanofski
Copy link
Member

@frasertweedale the problem is, that the actions are "too well encapsulated" at this point. For example:

  Keybinding (V.EvKey (V.KChar 'k') []) (listUp `chain'` displayMail `chain` continue)

performs the listUp first and then performs the displayMail action. The actions are distinct from each other so that displayMail which resets the scroll state doesn't know the previous action performed a listUp. What that means is, from the viewpoint of displayMail, you just know what the selected scroll state is, not what it was. You'd have to know that in order to compare. Unless we introduce some concept of Context passed on to Actions which encapsulates the state before the change or something.

@frasertweedale
Copy link
Member Author

@romanofski OK, so more work is needed then, e.g. a way to "abort" a chain of actions or something like that... just shelve this for now, I'll think about it some more :)

@romanofski romanofski modified the milestones: 1.0, Future Feature Apr 21, 2019
@frasertweedale frasertweedale modified the milestones: Future Feature, 2023.1 Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants