Skip to content

Conversation

@aryany9
Copy link

@aryany9 aryany9 commented Nov 4, 2024

No description provided.

@aryany9 aryany9 changed the title Added optional flag showToast. Added optional flag showToast. closes #2 Nov 4, 2024
@aryany9 aryany9 changed the title Added optional flag showToast. closes #2 Added optional flag showToast. Nov 4, 2024
@aryany9
Copy link
Author

aryany9 commented Nov 4, 2024

close #2

Copy link
Owner

@akshaydoshi2 akshaydoshi2 left a comment

Choose a reason for hiding this comment

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

Hey @aryany9,
Thanks for the PR! Your code works well, but I suggest an improvement for handling the showToast flag.
If showToast is set to false, it would be better to avoid creating the broadcast receivers and not passing them to the sendTextMessage method.
This way, we can reduce unnecessary overhead.

@aryany9 aryany9 requested a review from akshaydoshi2 November 4, 2024 11:11
@aryany9
Copy link
Author

aryany9 commented Nov 4, 2024

Hi @akshaydoshi2 , I reviewed your suggestion and tried to implement the same however, As per the roadmap we have to implement the event channel as well to receive the SMS status from the broadcast receivers which makes it necessary to be there irrespective of the showToast flag.

My latest commit on the branch implemented the event channel to receive the sms status. Hope so this will work as expected.

@akshaydoshi2
Copy link
Owner

Hey @aryany9,
Sounds good! Let me review and test it once.

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.

2 participants