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 authentication & reactions #31

Merged
merged 5 commits into from
Mar 10, 2020
Merged

Add authentication & reactions #31

merged 5 commits into from
Mar 10, 2020

Conversation

jvnm-dev
Copy link
Collaborator

@jvnm-dev jvnm-dev commented Feb 28, 2020

Added authentication & reactions

Documentation need to be updated to explain how to set this thing up (create a github app with write access on the blog repository, not OAuth app. And with homepage & callback = https://username.github.io/react-blog-github/)

Things to note:

  • userClient should be used for request things as the current user
  • Apollo GraphQL hooks are always done as the repository owner

Finally, it improves some things & delete unused files.

Extra note: I was forced to put https://username.github.io/react-blog-github/ as callback because github doesn't support # in urls

Working example: https://jvm-odoo.github.io/react-blog-github/#/

If you see any thing to improve do not hesitate to comment!

Related issue #26

@jvnm-dev jvnm-dev self-assigned this Feb 28, 2020
@jvnm-dev jvnm-dev added the enhancement New feature or request label Feb 28, 2020
@saadpasta
Copy link
Owner

@jvm-odoo Amazing Work 😀. I guess we should also update the readme.md with this that how can user create Github client secret and id and what should be the redirect url.

package.json Show resolved Hide resolved
return "";
}
}, []);
const reactionsContainer = useRef(null);
Copy link
Owner

Choose a reason for hiding this comment

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

Why useRef here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To force the reactions to refresh when adding/removing one

Copy link
Owner

Choose a reason for hiding this comment

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

OK, so we call API again to get the reactions or we update it manually

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I managed to use minimum API calls as possible in order to improve performances

Copy link
Owner

@saadpasta saadpasta left a comment

Choose a reason for hiding this comment

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

Look Good. I guess if you could update the readme.md with these changes and create a new version of the release.

@jvnm-dev
Copy link
Collaborator Author

jvnm-dev commented Mar 2, 2020

I will update the readme asap!

Copy link
Owner

@saadpasta saadpasta left a comment

Choose a reason for hiding this comment

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

LGTM😀

@saadpasta saadpasta merged commit 57cde86 into saadpasta:master Mar 10, 2020
@jvnm-dev jvnm-dev deleted the authentication-reactions-jvm branch March 10, 2020 06:52
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