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

feat : Create an example package for altfire_authenticator #71

Merged
merged 6 commits into from
Feb 14, 2024

Conversation

k-nkmr
Copy link
Contributor

@k-nkmr k-nkmr commented Feb 9, 2024

πŸ”— Issue Link

closes #17

πŸ™Œ What I did

✍️ What I didn't do

βœ… Verification

  • Android
  • iOS
  • macOS
  • Web

Screenshots

Before pressing the button After pressing the button
IMG_0014 IMG_0015

Additional Information

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Feb 9, 2024
Copy link

codecov bot commented Feb 9, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Comparison is base (110297e) 58.19% compared to head (72b0620) 58.19%.
Report is 1 commits behind head on main.

❗ Current head 72b0620 differs from pull request most recent head e95d94f. Consider uploading reports for the commit e95d94f to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #71   +/-   ##
=======================================
  Coverage   58.19%   58.19%           
=======================================
  Files          13       13           
  Lines         299      299           
=======================================
  Hits          174      174           
  Misses        125      125           

β˜” View full report in Codecov by Sentry.
πŸ“’ Have feedback on the report? Share it here.

@k-nkmr k-nkmr marked this pull request as ready for review February 13, 2024 08:58
@k-nkmr k-nkmr requested a review from a team as a code owner February 13, 2024 08:58
@k-nkmr k-nkmr requested review from naipaka and removed request for a team February 13, 2024 08:58
Comment on lines 30 to 41
FilledButton(
child: const Text('Sign in with Google'),
onPressed: () async {
await authenticator.signInWithGoogle();
},
),
FilledButton(
child: const Text('Sign out'),
onPressed: () async {
await authenticator.signOut();
},
),
Copy link
Contributor

@naipaka naipaka Feb 13, 2024

Choose a reason for hiding this comment

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

badge
Thank you for adding the button! πŸš€
How about switching the button according to the sign-in status, now that we have it? πŸ‘€
The idea is to display a sign-in button when not signed in, and a sign-out button when signed in ✍️
It would be nice for a sample if the uid is also displayed when signed in!

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 changed the UI based on whether the user is signed in or notπŸš€

Copy link
Contributor

@naipaka naipaka left a comment

Choose a reason for hiding this comment

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

I would like you to add a launch configuration for altfire_authenticator in launch.json, please!

ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon;
CLANG_ENABLE_MODULES = YES;
CURRENT_PROJECT_VERSION = "$(FLUTTER_BUILD_NUMBER)";
DEVELOPMENT_TEAM = 9DSUD2DLG2;
Copy link
Contributor

Choose a reason for hiding this comment

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

badge
It would be helpful if you could change DEVELOPMENT_TEAM to Altive Inc.!

Before After
γ‚Ήγ‚―γƒͺγƒΌγƒ³γ‚·γƒ§γƒƒγƒˆ 2024-02-13 19 48 09 γ‚Ήγ‚―γƒͺγƒΌγƒ³γ‚·γƒ§γƒƒγƒˆ 2024-02-13 19 48 18

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 changed the DEVELOPMENT_TEAM setting and updated the settings.jsonπŸ“

Comment on lines 28 to 29
final authenticator = ref.watch(authenticatorProvider);
final uid = ref.watch(uidProvider);
Copy link
Contributor

Choose a reason for hiding this comment

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

badge
Thank you for implementing the branching of button displays! πŸš€

badge
I think it's appropriate to minimize dependencies on other packages in public packages, so I would like to ask for an implementation without using riverpod!

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 removed Riverpod from the dependenciesπŸ“

Copy link
Contributor

@naipaka naipaka left a comment

Choose a reason for hiding this comment

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

LGTM
Thank you for the correction! LGTM! πŸš€

@k-nkmr k-nkmr merged commit 8902cbf into main Feb 14, 2024
4 checks passed
@k-nkmr k-nkmr deleted the 17-add-authenticator-example branch February 14, 2024 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create an example package for altfire_authenticator
2 participants