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

Further development of CSS Frameworks with JavaScript.

Includes JS:

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

Copy link

@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();

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";


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);

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.")

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";

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}`;

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>`;

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}/)}`;

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>`;

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>`;

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