zulip-terminal
zulip-terminal copied to clipboard
zulip
Typing Indicator Animation
This commit animates the typing feedback using an iterable cycle
object. This is done using an asynchronous function
_show_typing_animation and a boolean is_recipient_typing to
start and end the animation.
The corner case of not recieving a stop signal is handled by
setting TYPING_STARTED_EXPIRY_PERIOD=15 and checking this
condition using datetime.
This has to be handled differently from #915 as footer keep updating every half second
so, footer text's duration variable can't be used.
Tests added.
@preetmishra Yes, I am aware of the things specified that I have not added yet. This is just a functional prototype and since it's small enough to understand, I was planning to write a proper commit structure after making those changes. I am in between my exams right now hence the not so perfect PR. Hope you understand :)
@preetmishra I have made a few more changes to handle corner cases better. Even the tests work better now. (I have moved time forward instead of reducing it)
It all works but PEP8 tells me that there is a trailing whitespace error on a line I haven't even touched.
I even tried using VSCode's Trim Trailing Whitespace command.
zulipterminal/model.py:960:68: W291 trailing whitespace
It all works but PEP8 tells me that there is a trailing whitespace error on a line I haven't even touched.
You did change that line. You can take a look at the Files changed tab of this PR and notice that line 960 is changed. You may be in a subsequent commit while running the linter and thought that you didn't change that line. :sweat_smile:
(That's why it's important to run tests for each commit so that you find your mistakes in that commit itself.)
PRO-TIP: git blame can prove really helpful in finding out who changed a particular line in a repository.
@zee-bit Oh wait I was checking in a different file :sweat_smile: , I mistook model.py for test_model.py cause that was the only file I was editing at that time. :facepalm:
Yes, I use something similar to git blame :+1:
@preetmishra This is ready for another review. I have made all the suggested changes except the datetime refactor for test_ui_tools. Hopefully the rest of it is looking good?
@preetmishra I have updated the PR with all the suggestions. Hopefully this is now complete.
@neiljp Thanks for the review. To address some of your points:
- Yes this also happens to resolve the same edge case as @prah23's #915 , where the basic idea was borrowed from, as it had to be handled differently as this is asynchronous.
- Moving this whole function into controller makes sense and would simplify work on typing notifications features later.
- The full recipient was not needed, it was used it the same way before hence I kept it. Maybe just sending the name is better.
I have updated this PR as per suggestions and also have improved the tests.
@Rohitth007 Thanks for working on this different architecture - the first two commits of this are now merged :tada: (since the tests seem OK that far through)
I split the first commit to make it smaller and easier to read.
Heads up @Rohitth007, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.
