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

added fullstacksherpa folder in participants folder #20

Closed
wants to merge 0 commits into from

Conversation

fullstacksherpa
Copy link
Contributor

@fullstacksherpa fullstacksherpa commented Oct 12, 2023

Description

-integrate GET from API https://www.dnd5eapi.co/docs/, using https://developer.mozilla.org/en-US/docs/Web/HTTP

Changes Made

-added fullstacksherpa folder in participants folder and added fullstacksherpa list item in bootcamp.html

Related Issues

issue #13
Resolves #13

Checklist

  • I have tested these changes locally.
  • I have added/updated documentation as needed.
  • I have reviewed my code for potential issues.
  • I have followed the project's coding guidelines.
  • I have updated relevant comments and commit messages.

Additional Information

-all the style are done using tailwindcss framwork.
-fetching data from D&D5eAPI

By submitting this PR, I confirm that

  • I have read and understood the project's contributing guidelines.
  • This PR is ready for review and merging.
  • I understand that automated tests should pass before merging.

const apiUrl = `https://www.dnd5eapi.co/api/conditions/${selectedCondition}`;

// Make a GET request using fetch
fetch(apiUrl)
Copy link
Member

Choose a reason for hiding this comment

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

Can you try it with async await ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure,I will.

const descPoints = data.desc.map((point) => `<li>${point}</li>`).join("");

resultElement.innerHTML = `
<h3 class="text-center text-3xl">Description for the <span class="font-bold leading-4 tracking-widest text-purple-600">${selectedCondition} </span>condition</h3>
Copy link
Member

Choose a reason for hiding this comment

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

Do you have any alternative solution here? Instead of innerHTML method ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it okay to use document.createElement and appendChild instead of using innerHTML??

Copy link
Collaborator

@pfieffer pfieffer left a comment

Choose a reason for hiding this comment

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

As @chauchausoup suggested instead of the callbacks maybe using async await will be a better approach.
But looks good.
Thanks for the pr @fullstacksherpa

Copy link
Collaborator

@manjilkoirala manjilkoirala left a comment

Choose a reason for hiding this comment

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

Overall code seems good and working but as @chauchausoup suggested use async and wait as you are calling from some REST API.
Also, you can try not to add HTML syntax inside JS.
Thank you for the PR☺️

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.

[Task Assignment]: D&D 5e API Implementation
4 participants