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

Firebase web default initialization through dart code #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jmolins
Copy link
Contributor

@jmolins jmolins commented Apr 22, 2020

The plugin configuration includes the firebase configuration for web build right in dart code in the main.dartfile.
But this configuration is never used.
This PR modifies the web.dart file in the plugin authentication code so the firebase initialization is done through dart code if the <script> item is removed in the index.html.
The Readme.md file has been edited to explain this behavior.

@rodydavis
Copy link
Owner

Does this still work for desktop? I guess they recently added firebase_auth for macos because when I created this plugin it was need for the rest api wrapper I built

@jmolins
Copy link
Contributor Author

jmolins commented Apr 22, 2020 via email

@jmolins
Copy link
Contributor Author

jmolins commented Apr 24, 2020

Hi Rody,

The desktop rest-api still works in macOs.

And the "mobile" version works as well with the new firebase_auth for macos.

I have replaced:
static bool get isDesktop => Platform.isWindows || Platform.isLinux || Platform.isMacOS; static bool get isMobile => Platform.isIOS || Platform.isAndroid;
with:
static bool get isDesktop => Platform.isWindows || Platform.isLinux; static bool get isMobile => Platform.isIOS || Platform.isAndroid || Platform.isMacOS;
and it works using the "mobile" FbSdk class.

Maybe some renaming should be performed because with the new firebase_auth the distinction desktop/mobile is no longer true.

Do you want a PR trying the above code change and a rename?

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