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

Shortcuts for conversations #280

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

Choucroute-melba
Copy link

@Choucroute-melba Choucroute-melba commented Jan 8, 2025

What is it?

  • Bugfix
  • Feature
  • Codebase improvement

Description of the changes in your PR

  • Added ShortcutHelper class
  • Publishong shortcuts for each used thread

This changes add conversation shortcuts to the launcher. They also aim to integrate the app in the Android conversation space.

Fixes the following issue(s)

Acknowledgement

@Aga-C
Copy link
Member

Aga-C commented Jan 9, 2025

Thanks for contributing! I've tested it and found some problems:

  • New conversation shortcut is treated like a conversation and disappears at some point. It should be pinned to the top.
  • Private contacts are displayed as numbers.
  • Deleted conversations are still visible as conversation shortcuts.
  • Opening deleted conversations from a shortcut throws An unknown error occured. As we can't remove shortcuts from launcher, we can just open the app on the conversations list (e.g. Google Messages do it this way).
  • If you have multiple SMS apps (most likely some users won't turn off their preinstalled one), it asks in which app the shortcut should be opened. As it's a shortcut from Fossify Messages, it should always open Fossify Messages.

@naveensingh naveensingh added the waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. label Jan 9, 2025
Comment on lines 68 to 72
import org.fossify.messages.extensions.*
import org.fossify.messages.helpers.*
import org.fossify.messages.messaging.*
import org.fossify.messages.models.*
import org.fossify.messages.models.ThreadItem.*
Copy link
Member

Choose a reason for hiding this comment

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

Please do not wildcard imports. We are moving away from it.

try {
sanitizer.sanitizeByThrowing(intent)
} catch (e: Exception) {
toast(e.message ?: getString(org.fossify.commons.R.string.unknown_error_occurred))
Copy link
Member

Choose a reason for hiding this comment

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

Just use the showErrorToast(e) extension like everywhere else. Better exception handling is planned but this is not the way.

Comment on lines 332 to 340
val draft = try {
draftsDB.getDraftById(id)
} catch (e: Exception) {
if (e.message == "Cannot access database on the main thread since it may potentially lock the UI for a long period of time.") {
null
} else {
throw e
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This code is always supposed to be called in a background thread.

Also, that's not a good way to catch an exception.

@Choucroute-melba
Copy link
Author

Hi, everything you've mentionned should be ok now except for your second point @Aga-C , I don't know about private contacts so i've tried something but if it's not working please could you give me more details on how to handle this ?
Also about import wildcards it's done automatically by the ide, .editorconfig need to be changed to avoid this
Thanks for reviewing my work, have a nice day.

@Aga-C Aga-C removed the waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. label Jan 10, 2025
@Aga-C
Copy link
Member

Aga-C commented Jan 10, 2025

I don't know about private contacts so i've tried something but if it's not working please could you give me more details on how to handle this ?

Private contacts are contacts stored in Fossify Contacts as not visible for other apps (but still can be accessed by other Fossify apps). It's still not working. Probably that's because for the title you rely only on conversation title. See how it's done inside the app to ensure that the correct title is always displayed:

private fun setupThreadTitle() {
val title = conversation?.title
binding.threadToolbar.title = if (!title.isNullOrEmpty()) {
title
} else {
participants.getThreadTitle()
}
}

Also, I've found some more bugs while testing:

  • When I add a shortcut to the home screen, the last character from the name (or phone number) is missing.
  • We can change conversation name in Conversation Details. It's not taken into account, the original name is always shown in the shortcuts menu.

@Choucroute-melba
Copy link
Author

Hi again, I was able to do necessary changes. Tell me if there is anything else I can do

@Aga-C
Copy link
Member

Aga-C commented Jan 11, 2025

Seems that everything works fine, thanks!

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.

Feat - Conversation Shortcuts in Launcher
3 participants