Awesome Book: Using modules with ES6 #1

miltonHenschel · 2022-12-13T06:49:41Z

Use ES6 syntax and arrow functions
Divide code into modules
Display the current date below the navigation bar.

Olamarx

Good job so far, indeed you have spent time on this project!
There are some issues that you still need to work on to go to the next project but you are almost there!

Highlights

All linters passed ✔️
Add and remove ✔️

Required Changes ♻️

Check the comments under the review.

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

Olamarx · 2022-12-13T08:01:52Z

modules/localStore.js

+    return books;
+  }
+
+  static addBooks(newBook) {


Kindly make use of ES6 arrow function for your approach to be of latest and it would help you in React module.

Please @Olamarx I didn't succeed to change the static functions to arrow functions. The last I checked, it's not possible to do that in classes. 😁 Please could you help me to achieve that? Thanks in advance.

Olamarx · 2022-12-13T08:02:24Z

modules/ui.js

+let newId = 0;
+
+class UI {
+  static showBooks() {


Kindly make use of the ES6 arrow function for your approach to be of the latest and it would help you in React module. Apply this to others too, please.

Olamarx · 2022-12-13T08:12:58Z

js/index.js

+const timeDisplay = () => {
+  const timeInfo = document.querySelector('#time-info');
+  timeInfo.innerHTML = `${DateTime.now().toLocaleString(DateTime.DATETIME_MED)}`;
+};


This function is not working. Kindly take a second look. You can use += instead of = alone. Just a suggestion.

Olamarx

Hi @miltonHenschel,

Good job so far, indeed you have spent time on this project!
There are some issues that you still need to work on to go to the next project but you are almost there!

Highlights

All linters passed ✔️
Add and remove ✔️

Required Changes ♻️

Check the comments under the review.

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

Gopxfs

♻️ STATUS: REQUIRED CHANGES ♻️

Hi @miltonHenschel 👋,

You've done well implementing some of the requested changes 👏 , but there are still some which haven't been addressed yet.

Highlights 🚀

✔️ No linter errors.
✔️ Great code structure. Well done!
✔️ Implemented some of the changes requested by the previous reviewer.

Required Changes ♻️

Check the comments under the review.

Optional suggestions 🗒️

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you take them into account as they can make your code better.

Cheers and Happy coding!👏👏👏

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me @Gopxfs in your question so I can receive the notification.

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

Gopxfs · 2022-12-13T15:57:25Z

modules/localStore.js

+  static getBooks() {
+    let books;
+    const localBook = localStorage.getItem('local');
+    if (!localBook) {
+      books = [];
+    } else {
+      books = JSON.parse(localBook);
+    }
+    return books;
+  }
+
+  static addBooks(newBook) {


As requested by the previous reviewer, please use arrow functions instead of normal functions 😉.
Tip: you could have something like this:

Check this link to learn more about arrow functions.

As you are using static functions, adapting the code I showed above may introduce you to new errors or even stop your code from working 😄. To overpass that, try this code:

It will probably work as you expect and avoid unintended errors while using the modern syntax 😉.

Gopxfs · 2022-12-13T15:57:40Z

modules/ui.js

+class UI {
+  static showBooks() {
+    const books = LocalStore.getBooks();
+    books.forEach((newBook) => {


Please also make sure to update this class methods.

GzimAsani

STATUS: APPROVED 💯

There is nothing else to say than it's time to merge... 🥳

Cheers and Happy coding! 👏 💻 🍷
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.

Optional suggestions

Every comment with the [OPTIONAL] prefix is not crucial enough to stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better.

You can also consider:

N/A

Please be reminded to not open a new pull request after you get a review, only use the same one 💭

Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.

_As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

brytebee

Hi, @miltonHenschel👋,

Highlights💡

Good commit message.✔️
Good PR Summary.✔️
Great UI design.✔️
App functions as it should.✔️

STATUS: APPROVED 🕺🥳

Congratulations🍾🥳!! Your project is ready for presentation! There is nothing else to say other than... it's time to merge it

Optional Suggestions:

N/A

Good luck! 🎉

Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me @brytebee in your question so I can receive the notification.

Use @brytebee on slack for faster responses.

As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

brytebee · 2023-01-16T15:15:17Z

modules/localStore.js

+    }
+  }
+
+  // static deleteBooks()


Optional: kindly remove unused codes.

brytebee · 2023-01-16T15:17:00Z

js/index.js

+    books = books.filter((bk) => JSON.stringify(bk.id) !== id);
+    localStorage.setItem('local', JSON.stringify(books)); // LOCAL STORAGE
+    UI.deleteBookFromList(e.target);
+    // window.location.reload();


Optional: kindly remove unused codes throughout this file.

miltonHenschel added 6 commits December 13, 2022 06:58

Setup add/remove buttons

6a7c7de

Create book class in book.js and import it to index.js

9386dab

Create UI class in ui.js and export to index.js

dd3c306

Create LocalStore class in localStore.js and export to index.js

0393385

Create MenuNav class in menuNav.js and export to index.js

b4f7f5d

Import time and date library from luxon.js and export to index.js

05df73d

miltonHenschel temporarily deployed to github-pages December 13, 2022 06:50 — with GitHub Pages Inactive

Fix linter errors from luxon.js

ca39b56

miltonHenschel temporarily deployed to github-pages December 13, 2022 07:25 — with GitHub Pages Inactive

Olamarx suggested changes Dec 13, 2022

View reviewed changes

Updae luxon.js to use ES6

045eea1

miltonHenschel temporarily deployed to github-pages December 13, 2022 08:53 — with GitHub Pages Inactive

remove link to btnAddClick js file from html

cd46796

miltonHenschel temporarily deployed to github-pages December 13, 2022 14:44 — with GitHub Pages Inactive

Gopxfs suggested changes Dec 13, 2022

View reviewed changes

Refactor UI and LocalStore to use arrow functions

ec4fd87

miltonHenschel temporarily deployed to github-pages December 14, 2022 12:57 — with GitHub Pages Inactive

miltonHenschel added 2 commits December 14, 2022 21:00

Add media query for desktop version

acd155e

Fix css linter error

166d63e

miltonHenschel deployed to github-pages December 14, 2022 20:06 — with GitHub Pages View deployment

GzimAsani reviewed Dec 19, 2022

View reviewed changes

brytebee approved these changes Jan 16, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Awesome Book: Using modules with ES6 #1

Awesome Book: Using modules with ES6 #1

miltonHenschel commented Dec 13, 2022

Olamarx left a comment

Olamarx Dec 13, 2022

miltonHenschel Dec 13, 2022

Olamarx Dec 13, 2022

Olamarx Dec 13, 2022 •

edited by miltonHenschel

Olamarx left a comment

Gopxfs left a comment

Gopxfs Dec 13, 2022

Gopxfs Dec 13, 2022

GzimAsani left a comment

brytebee left a comment

brytebee Jan 16, 2023

brytebee Jan 16, 2023

Awesome Book: Using modules with ES6 #1

Are you sure you want to change the base?

Awesome Book: Using modules with ES6 #1

Conversation

miltonHenschel commented Dec 13, 2022

Olamarx left a comment

Choose a reason for hiding this comment

Highlights

Required Changes ♻️

Optional suggestions

Olamarx Dec 13, 2022

Choose a reason for hiding this comment

miltonHenschel Dec 13, 2022

Choose a reason for hiding this comment

Olamarx Dec 13, 2022

Choose a reason for hiding this comment

Olamarx Dec 13, 2022 • edited by miltonHenschel

Choose a reason for hiding this comment

Olamarx left a comment

Choose a reason for hiding this comment

Highlights

Required Changes ♻️

Optional suggestions

Gopxfs left a comment

Choose a reason for hiding this comment

♻️ STATUS: REQUIRED CHANGES ♻️

Hi @miltonHenschel 👋,

Highlights 🚀

Required Changes ♻️

Optional suggestions 🗒️

Gopxfs Dec 13, 2022

Choose a reason for hiding this comment

Gopxfs Dec 13, 2022

Choose a reason for hiding this comment

GzimAsani left a comment

Choose a reason for hiding this comment

STATUS: APPROVED 💯

There is nothing else to say than it's time to merge... 🥳

Optional suggestions

Please be reminded to not open a new pull request after you get a review, only use the same one 💭

brytebee left a comment

Choose a reason for hiding this comment

Highlights💡

STATUS: APPROVED 🕺🥳

Optional Suggestions:

Use @brytebee on slack for faster responses.

brytebee Jan 16, 2023

Choose a reason for hiding this comment

brytebee Jan 16, 2023

Choose a reason for hiding this comment

Olamarx Dec 13, 2022 •

edited by miltonHenschel