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

Matrix client improvements #149

Merged
merged 12 commits into from Dec 5, 2018
Merged

Matrix client improvements #149

merged 12 commits into from Dec 5, 2018

Conversation

derivmug
Copy link
Collaborator

@derivmug derivmug commented Dec 5, 2018

This PR adds a few things to the matrix client:

  • Display leave messages
  • Option to show/hide leave/join messages
  • Add list of room members on pressing RIGHT in the room menu
  • Add option to write a message to the bottom of each chat

@codecov
Copy link

codecov bot commented Dec 5, 2018

Codecov Report

Merging #149 into devel will increase coverage by 0.04%.
The diff coverage is 15.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##            devel     #149      +/-   ##
==========================================
+ Coverage   40.57%   40.62%   +0.04%     
==========================================
  Files         243      244       +1     
  Lines       19215    19393     +178     
==========================================
+ Hits         7797     7878      +81     
- Misses      11418    11515      +97
Impacted Files Coverage Δ
apps/matrix/main.py 17.07% <15.62%> (-1.37%) ⬇️
splash.py 7.69% <0%> (-0.31%) ⬇️
apps/system_apps/systemctl/systemctl.py 4% <0%> (ø)
ui/tests/test_menu.py 86.27% <0%> (+1.08%) ⬆️
apps/settings/main.py 33.98% <0%> (+7.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0bc88ec...7353f48. Read the comment docs.

Copy link
Member

@CRImier CRImier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from some small problems, all is good!

Menu(mc, self.i, self.o, catch_exit=False, name="Matrix app settings menu").activate()
mc = [
["Log out", self.logout],
["Hide join/leave messages", lambda: self._change_setting("show_join_leave_messages", False)],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, re-do this to toggle the setting instead (use contents_hook for autogenerating the correct label), so that there's only one entry instead of two - and it also shows the status

apps/matrix/main.py Show resolved Hide resolved
@@ -230,7 +273,11 @@ def _on_message(self, room, event):
prefix = ""
if event.get('sender', None) == self.client.get_user().user_id or event.get("sender", None) in self.config["your_other_usernames"]:
# Prefix own messages with a '*'
prefix = "* "
prefix = "[*] "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you want to add two more characters? We don't have too many of them on the screen =)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was wondering about this as well, I'll change it back to one =)


def gen_menu_contents():
mc = [["Log out", self.logout]]
mc.append(["Hide join/leave messages", lambda: self._change_setting("show_join_leave_messages", False)] if self.config["show_join_leave_messages"] else ["Show join/leave messages", lambda: self._change_setting("show_join_leave_messages", True)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line is too long, we can shorten it like this:

Suggested change
mc.append(["Hide join/leave messages", lambda: self._change_setting("show_join_leave_messages", False)] if self.config["show_join_leave_messages"] else ["Show join/leave messages", lambda: self._change_setting("show_join_leave_messages", True)])
conf_key = "show_join_leave_messages"
mc.append([ "Hide join/leave messages", \
lambda: self._change_setting(conf_key, False) ] \
if self.config[conf_key] else [ "Show join/leave messages", \
lambda: self._change_setting(conf_key, True) ])

Do test that it works after merging if you can, please =)

apps/matrix/main.py Show resolved Hide resolved
@CRImier CRImier merged commit 3801822 into devel Dec 5, 2018
@CRImier
Copy link
Member

CRImier commented Dec 5, 2018

Thank you!

@CRImier CRImier deleted the matrix branch December 5, 2018 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants