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: dynamic clientId Initialization #61

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

zuhno
Copy link
Contributor

@zuhno zuhno commented Jan 2, 2024

Hi :)

  1. Added folders for methods, constants, and states (for global ref) under src, and restructured the folder architecture considering scalability for future method development.
  2. Introduced a new method named setGoogleClientId, which operates by changing the global ref value.
  3. The clientId can be initialized in the following ways:
  • Statically during middleware installation.
  • Dynamically in any component.
  • Initially set and later updated dynamically.
  1. Fixed an issue where the style height value was not being applied to the element before the renderButton of GoogleSignInButton is called.
  2. Updated the documentation, including a caution block in the section about initializing nuxt-vue3-google-signin.

I am very happy to contribute to this project. Please let me know if there are any suggestions for improvements in the folder structure or functionality.

@kasvith
Copy link
Collaborator

kasvith commented Jan 2, 2024

Thanks @zuhno

this is a brilliant work, and im really happy about this PR 💪
i will review the code and let you know my thoughts :)

it looks really good as i checked on surface

@kasvith kasvith self-requested a review January 2, 2024 16:06
@kasvith kasvith marked this pull request as draft January 2, 2024 16:06
@kasvith kasvith changed the title Enhancement: Dynamic clientId Initialization feat/Dynamic clientId Initialization Jan 2, 2024
@kasvith kasvith added the enhancement New feature or request label Jan 2, 2024
@kasvith kasvith marked this pull request as ready for review January 2, 2024 16:07
@zuhno zuhno mentioned this pull request Jan 15, 2024
Copy link
Collaborator

@kasvith kasvith left a comment

Choose a reason for hiding this comment

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

im going through the code little bit

@zuhno
Copy link
Contributor Author

zuhno commented Apr 26, 2024

@kasvith Please let me know if there are any parts that need to be revised.

@kasvith
Copy link
Collaborator

kasvith commented Jun 17, 2024

Sure, i didnt have much time lately to focus on the project

@kasvith kasvith changed the title feat/Dynamic clientId Initialization feat: dynamic clientId Initialization Jun 17, 2024
@kasvith kasvith self-requested a review October 31, 2024 15:38
src/utils/validations.ts Outdated Show resolved Hide resolved
refactor: applied early return to 'isClientIdValid'
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets rename this as src/store.ts

Copy link
Contributor Author

@zuhno zuhno Nov 21, 2024

Choose a reason for hiding this comment

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

@kasvith
yes, the changes have been made as follows:

  • identified the similar structure of states and constants and relocated them to src/store.ts and src/constant.ts, respectively.
  • converted functions from expressions to declarations to ensure consistency with the existing code style.
  • added detailed jsdoc comments to the setGoogleClientId method to improve maintainability and provide clear usage instructions.

@kasvith
Copy link
Collaborator

kasvith commented Nov 21, 2024

hi @zuhno made a comment

zuhno and others added 2 commits November 21, 2024 16:19
@kasvith kasvith added this to the v3.0.0 milestone Nov 21, 2024
@kasvith
Copy link
Collaborator

kasvith commented Nov 21, 2024

im adding this to the v3 milestone

@zuhno
Copy link
Contributor Author

zuhno commented Nov 21, 2024

I have checked this. I'll check the support for this feature in nuxt.
thank you for reviewing the PR :)

@kasvith
Copy link
Collaborator

kasvith commented Nov 21, 2024

@zuhno we already have a nuxt module, it needs a rewrite with newer nuxt kit

once this is merged we can work on that

@zuhno
Copy link
Contributor Author

zuhno commented Nov 22, 2024

i have checked the related repository and will follow up on the merge status. thanks.

@kasvith
Copy link
Collaborator

kasvith commented Nov 22, 2024

Now all looks great, i will check this out locally and then we are good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants