Skip to content

Conversation

@rayzeller
Copy link
Contributor

@rayzeller rayzeller commented Mar 8, 2021

🔹 JIRA Ticket(s) if any

✏️ Description

Add setReadForMessage to Android bridge

image

@roninopf
Copy link
Contributor

roninopf commented Mar 8, 2021

Hi @rayzeller, thanks for the PR, but we're fixing the message read state very soon. Was there a particular use for this you had that wasn't mentioned in this issue report? #92

@rayzeller
Copy link
Contributor Author

I think when read is integrated into an API call, it will allow us to refactor our code so that we don't need to use the above method.

However, right now setReadForMessage is only defined for iOS devices. Trying to use it on Android would cause our devices to crash. I went ahead and PR-ed the patch we made so this wouldn't be the case. I guess we could also skip this API altogether with set(message, 'read', true) or a similar call, but right now the Typescript for setReadForMessage is misleading.

@rayzeller
Copy link
Contributor Author

Also @jehartzog and I work at the same company on a team of 3 engineers. Glad you're working on the issue ^_^. You have anything to add Eric?

@jehartzog
Copy link
Contributor

Nothing to add. Glad to hear there may be plans to transfer read field support to the API, that will enable the use cases we originally planned on. I agree w/ merging this PR in the interim, as using it currently will cause crashes on Android devices.

@roninopf
Copy link
Contributor

roninopf commented Mar 9, 2021

I think when read is integrated into an API call, it will allow us to refactor our code so that we don't need to use the above method.

However, right now setReadForMessage is only defined for iOS devices. Trying to use it on Android would cause our devices to crash. I went ahead and PR-ed the patch we made so this wouldn't be the case. I guess we could also skip this API altogether with set(message, 'read', true) or a similar call, but right now the Typescript for setReadForMessage is misleading.

AH! Thank you. Another team member also informed me of this as well, I didn't realize this was happening since I usually work on the iOS bridge.

@roninopf roninopf self-requested a review March 9, 2021 00:39
@roninopf roninopf merged commit 4d6c1bf into Iterable:master Mar 10, 2021
roninopf added a commit that referenced this pull request Apr 1, 2021
…android-bridge

Add setReadForMessage to Android bridge
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.

3 participants