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

Add workaround for firebase messaging #289

Merged
merged 11 commits into from Jan 24, 2022

Conversation

ikbalkaya
Copy link
Contributor

@ikbalkaya ikbalkaya commented Jan 7, 2022

This PR adds a warning for users who use Firebase messaging library along with Ably library

@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 7, 2022 15:53 Inactive
@ikbalkaya ikbalkaya added the documentation Improvements or additions to public interface documentation (API reference or readme). label Jan 7, 2022
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 7, 2022 15:57 Inactive
@ikbalkaya ikbalkaya self-assigned this Jan 7, 2022
@ikbalkaya ikbalkaya marked this pull request as ready for review January 7, 2022 15:57
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Co-authored-by: Quintin Willison <q@qwuk.net>
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 7, 2022 16:57 Inactive
Co-authored-by: Quintin Willison <q@qwuk.net>
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 7, 2022 17:03 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 7, 2022 17:11 Inactive
README.md Outdated
@@ -88,6 +88,16 @@ Under the run/ debug configuration drop down menu, click `Edit Configurations...

See [PushNotifications.md](PushNotifications.md) for detailed information on getting PN working with the example app.

#### [Important Note regarding Firebase and Push Notifications](https://github.com/ably/ably-flutter/issues/226)
Copy link
Contributor

Choose a reason for hiding this comment

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

This readme actually has two sections with the same "Push Notifications" heading. 🙄
This one, and another one later on.

Perhaps that was the initial impetus for the creation of the dedicated, discrete document PushNotifications.md.

Regardless of reasons, the reality is that there are two places that could do with this information being visible... therefore my suggestion would be to move this new content to the dedicated document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was aware of that but I thought this would be a bit more prominent if we have this on the original readme. And I also think that the readme here is in needs of shortening, Perhaps we can then move push notification section here. But I will move this to the separate file for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

README.md Outdated
@@ -88,6 +88,16 @@ Under the run/ debug configuration drop down menu, click `Edit Configurations...

See [PushNotifications.md](PushNotifications.md) for detailed information on getting PN working with the example app.

#### [Important Note regarding Firebase and Push Notifications](https://github.com/ably/ably-flutter/issues/226)

If you are using the [official Firebase messaging library package](https://pub.dev/packages/firebase_messaging) along with this library, then you must add the following block to your Android application's manifest file within the `application` element. This is a workaround that prevents a conflict rising from the two libraries installed together.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you are using the [official Firebase messaging library package](https://pub.dev/packages/firebase_messaging) along with this library, then you must add the following block to your Android application's manifest file within the `application` element. This is a workaround that prevents a conflict rising from the two libraries installed together.
If you are using the [official Firebase messaging library package](https://pub.dev/packages/firebase_messaging) along with this library, then you must add the following block to your Android application's manifest file within the `application` element. This is a workaround that prevents a conflict that happens when the two libraries are installed together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@QuintinWillison
Copy link
Contributor

Sorry, @ikbalkaya, I've spotted things on my second pass of this PR that I should have spotted the first time around... including something I introduced in one of my suggestions. My bad! 🤦

@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 14, 2022 11:18 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 14, 2022 11:22 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/289/dartdoc January 24, 2022 14:50 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to public interface documentation (API reference or readme).
Development

Successfully merging this pull request may close these issues.

Update Readme to provide workaround for clients using firebase_messaging library
3 participants