Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix for updating disappearing message settings plus a couple of drive-by fixes #138

Merged
merged 4 commits into from
Jun 9, 2021

Conversation

oxtoacart
Copy link

Closes getlantern/engineering#623.

@oxtoacart oxtoacart requested a review from cosmicespresso June 9, 2021 12:28
call.argument("displayName")!!
)
"setDisappearSettings" -> messaging.setDisappearSettings(
call.argument<String>("contactId")!!.directContactPath,
Copy link
Author

Choose a reason for hiding this comment

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

This is the actual fix, we have to pass the direct contact path.

attachments = call.argument<List<ByteArray>>("attachments")?.map { Model.StoredAttachment.parseFrom(it) }?.toTypedArray(),
replyToId = call.argument("replyToId"),
replyToSenderId = call.argument("replyToSenderId"))
call.argument("identityKey")!!,
Copy link
Author

Choose a reason for hiding this comment

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

Ran ktlintformat

@@ -502,10 +498,11 @@ class _ConversationState extends State<Conversation>
// TODO: generalize in theme
showRecentsTab: true,
recentsLimit: 28,
noRecentsText: 'No Recents',
noRecentsStyle: TextStyle(fontSize: 16, color: Colors.black26),
noRecentsText: 'No Recents'.i18n,
Copy link
Author

Choose a reason for hiding this comment

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

Using translated string.

// TODO: generalize in theme
categoryIcons: CategoryIcons(),
categoryIcons: const CategoryIcons(),
Copy link
Author

Choose a reason for hiding this comment

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

This and some of the other const declarations are linter fixes.

@@ -6,7 +6,6 @@ import 'package:focused_menu/modals.dart';

import 'package:lantern/messaging/messaging_model.dart';
import 'package:lantern/messaging/widgets/message_utils.dart';
import 'package:lantern/ui/widgets/copied_text_widget.dart';
Copy link
Author

Choose a reason for hiding this comment

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

Linter.

Choose a reason for hiding this comment

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

btw I haven't deleted that widget yet since I'm not sure where we will land with the message actions popup UX.

@@ -254,7 +251,6 @@ class _ConversationState extends State<Conversation>
_send(_newMessage.value.text,
replyToSenderId: _quotedMessage?.senderId,
replyToId: _quotedMessage?.id);
dismissKeyboard();
Copy link
Author

Choose a reason for hiding this comment

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

This was redundant with the dismissKeyboard() inside _send.

@@ -110,7 +108,6 @@ class _ConversationState extends State<Conversation>
replyToId: replyToId,
replyToSenderId: replyToSenderId,
);
dismissKeyboard();
Copy link
Author

Choose a reason for hiding this comment

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

When conversing with someone, I was annoyed by having to always re-focus on the message field to bring up the keyboard again. I think it's a more normal experience for the keyboard to just remain open so that you can keep firing off messages, so I took this out. @Kallirroi @Derekf5 please chime in if you disagree :)

Copy link
Author

Choose a reason for hiding this comment

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

To be precise, the text messaging app that I use and Signal both keep the text field focused and the keyboard open after sending a message.

Copy link

Choose a reason for hiding this comment

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

agreed! the keyboard should remain open after sending messages until the user actively dismisses it by hitting the native close keyboard button or clicking in the message window above the keyboard.

Choose a reason for hiding this comment

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

Sounds good to me - I might open a ticket specifically for nailing down the keyboard UX, as I think there's a few other glitches as well.

Copy link

@cosmicespresso cosmicespresso left a comment

Choose a reason for hiding this comment

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

  • just pushed an update to photo_manager (wouldn't run otherwise for me)
  • does this need the latest updates to messaging-android to run? Right now the duration selection menu behaves the same as before for me 🤔

@oxtoacart
Copy link
Author

  • Right now the duration selection menu behaves the same as before for me

Did you stop and recompile the app from scratch? The change was on the Kotlin side.

@oxtoacart
Copy link
Author

  • just pushed an update to photo_manager (wouldn't run otherwise for me)

I don't see an update?

@cosmicespresso
Copy link

  • just pushed an update to photo_manager (wouldn't run otherwise for me)

I don't see an update?

  • Right now the duration selection menu behaves the same as before for me

Did you stop and recompile the app from scratch? The change was on the Kotlin side.

Sorry - I ran into a pretty substantial git spaghetti situation 😬 Just sorted it out - everything works fine now! We can merge this.

@oxtoacart oxtoacart merged commit 131c71e into ox/messaging Jun 9, 2021
@oxtoacart oxtoacart deleted the ox/issue137 branch June 9, 2021 15:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants