-
Notifications
You must be signed in to change notification settings - Fork 26
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
Implement internationalization PapercupsIntl()
+ some fixes
#77
Conversation
Replace the new overscroll.disallowGlow() by the (soon-to-be) deprecated overscroll.disallowGlow() as the former is gonna be introduced in Flutter 2.5.6 (not out yet).
This looks awesome 😎 TYSM, I'll review it ASAP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -30,7 +30,8 @@ PhoenixChannel? joinConversationAndListen({ | |||
conversation = null; | |||
} else { | |||
if (event.event.toString().contains("shout") || | |||
event.event.toString().contains("message:created")) { | |||
event.event.toString().contains("message:created") || | |||
event.payload!["type"] == "reply") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a great catch!
I'll merge this now. However, I will not make a release quite yet because there are some kinks in the file upload I want to iron out, specifically want the upload to happen inside of |
closes #67 |
Regarding a major version release, I agree that this being a breaking PR it should be released as a major version in order to ensure minimal issues. We can (maybe?) pair it with attachments, although availability should be able to be implemented now too, I'll eventually get to it but right now I have no time to work on it. Hopefully will get a chance in the next month or so, or maybe you wanna take a stab at it? Let me know and I'll assign it, @CharlesMangwa. |
Thanks for the quick merge @aguilaair! And yes, I can look into Availability. I'd just need a little bit of guidance as I'm not really familiar with the API and even after looking in the Presence docs, I didn't quite get what I'm supposed to get 😅 But I guess we can continue this convo in #65. |
Hi @aguilaair!
This PR implements a couple of things:
PapercupsIntl()
class that brings translations for all strings I was able to find in the source code. One can now use it as such:This is a breaking change as I've removed the duplicated properties from both
Props
andPapercupsWidget
. My rationale is that this could be valid enough for a major release if coupled with the Availability feature for instance. Let me know If you'd rather have it in a minor release so that I add deprecation notices a little bit everywhere.A fix for a bug I found when running apps in
profile
orrelease
mode:event.event.toString()
only returns"PhoenixChannelEvent"
in those modes. So this case was nevertrue
papercups_flutter/lib/utils/socket/joinConversation.dart
Lines 32 to 33 in 95252be
which means that whenever a message was sent from the dashboard to an app, it wouldn't show up in real-time.
Replacing the use of Papercups
fullName
fordisplayName
as that seems more appropriate in this context.A couple of minor typos corrections here and there.