Skip to content

Js2#287

Open
renahashimi wants to merge 208 commits intoNoroffFEU:mainfrom
renahashimi:js2
Open

Js2#287
renahashimi wants to merge 208 commits intoNoroffFEU:mainfrom
renahashimi:js2

Conversation

@renahashimi
Copy link
Copy Markdown

Further development of CSS Frameworks with JavaScript.

Includes JS:

  • Create
  • Edit
  • Delete
  • API / Profile & Feeds

Copy link
Copy Markdown

@Paris2020 Paris2020 left a comment

Choose a reason for hiding this comment

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

Hi Rena!

Thank you for your submission.

FEEDBACK:

  • On unsuccessful login the user is still directed to the /feed/posts/ (see screenshot).
  • Profile and JWT token is not stored in LocalStorage (see screenshot).
  • User can’t create post (see screenshot).
  • See detailed feedback in PR.

Unfortunately this cannot go through as a pass

Kind regards
N Moseki

body
});

const { accessToken, ...user } = await response.json();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Object destructuring - good job!

const method = "post";
const action = "/auth/login";


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missed opportunity for JSDocs.

export async function registerUser(profile){
const registerUrl = `${API_SOCIAL_URL}${API_AUTH}${API_REGISTER}`;
const body = JSON.stringify(profile);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. There should be a try/catch here.

  2. A conditional statement to check if response is OK.

});

const result = await response.json();
alert("You are now registered. Please log in to your account.")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This alert() is very basic and should have been inside a try/catch block


const method = "post";
const author = "?_author=true";
//const action = "/posts";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Commented out lines should be removed from final code - line 7.


// USERNAME
const userName = document.createElement("h2");
userName.innerHTML = `${postData.owner}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Refrain from using innerHTML for templating rather use document.createElement.


const openFormBtn = document.createElement("button");
openFormBtn.classList.add("fw-bold", "m-2", "fs-5", "bg-secondary", "border", "border-black", "border-2")
openFormBtn.innerHTML = `Add a comment <i class="bi bi-chat-right-quote"></i>`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Refrain from using innerHTML for templating rather use document.createElement.


const commentDate = document.createElement("time");
commentDate.classList.add("d-block", "mt-n2", "fs-7")
commentDate.innerHTML = `${commentData.created.match(/^\d{4}-\d{2}-\d{2}/)}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Refrain from using innerHTML for templating rather use document.createElement.

} else if (isLiked) {
likeCountValue--;
likeCount.textContent = `${likeCountValue} Stars`;
likeBtn.innerHTML = `<i class="bi bi-star"></i>`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Refrain from using innerHTML for templating rather use document.createElement.

likeCountValue++;
likeCount.textContent = `${likeCountValue + storage.load(`liked_${postId}`)} Stars`;
likeBtn.classList.add("liked");
likeBtn.innerHTML = `<i class="bi bi-star-fill text-secondary"></i>`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Refrain from using innerHTML for templating rather use document.createElement.

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.

2 participants