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

Initial working site implementation. #60

Merged
merged 21 commits into from Nov 23, 2022
Merged

Initial working site implementation. #60

merged 21 commits into from Nov 23, 2022

Conversation

MichaelPachec0
Copy link
Owner

@MichaelPachec0 MichaelPachec0 commented Aug 9, 2022

This PR will merge into the master branch a working implementation of the site. This is still a WIP as i have not committed the whole site and (The code has been committed already) There is still the question of what to do when the user gets a full score. (will be implemented in another PR). For the moment the following things are functional:

  • Back-end loads.
  • Proper Webpage (along with css/js/img) gets sent to the client when requested, the modal (thanks to @thecornisians) is also here with proper attribution.
  • The api works and when a new game is requested and when you need to compare cards (for the moment it will only compare urls as the implementation was simple enough to create, this can be changed in the future).
  • Node packages have been frozen in anticipation of deploying on a Provider.
  • Fixes Update node-fetch #59 where an version of node-fetch was vulnerable, in regards to this web app this is not something that is affected by this, but there is no point in carrying a known vulnerable package, and i have tested the new version to work.
  • Fixes REFACTOR: Event handler should be contained in a function. #48 with a new async function grabCards and a Handler function called cardHandler.
  • Fixes Logic to keep score #28 and keeps score locally (The process to add to the score is done on the server so there are still requests done). Is possible if wanted to make the score be completely server side.

@MichaelPachec0
Copy link
Owner Author

MichaelPachec0 commented Aug 9, 2022

As of now this is all the work i have done. The last item still needs to be done for which i will try to get to today.

EDIT: In the future i might change a portion of the api to use a POST request (for example checking if the user should get a point)

EDIT 2: I will probably limit the pr to this and create the another that handles the winning condition in another. This already has a ton of changes that will make it difficult to review.

@MichaelPachec0 MichaelPachec0 marked this pull request as ready for review August 12, 2022 02:40
Copy link
Collaborator

@r-Dev03 r-Dev03 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@r-Dev03 r-Dev03 left a comment

Choose a reason for hiding this comment

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

LGTM, one thing, however, from what I recall the API returns an ID for each pokemon card. in the form of an array of objects, each object containing data on the card, and one of those properties being an ID.

@MichaelPachec0
Copy link
Owner Author

MichaelPachec0 commented Aug 12, 2022

LGTM, one thing, however, from what I recall the API returns an ID for each pokemon card. in the form of an array of objects, each object containing data on the card, and one of those properties being an ID.

The only problem is the client does not retain that info. I could have made that change (keeping the id) either keeping in JavaScript or putting it as a element attribute, but as you can imagine it would have created more changes (and more code commits). Id be more than happy to create another issue (or have anyone else create an issue tracking this) but i would want to keep that seperate to this PR.

@MichaelPachec0 MichaelPachec0 merged commit 892b7be into master Nov 23, 2022
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.

Update node-fetch REFACTOR: Event handler should be contained in a function. Logic to keep score
2 participants