Add DOM homework #392

misshatsker · 2022-09-01T17:55:18Z

DOM

Demo |
Code base

The code is submitted in a dedicated feature branch.

Only code files are submitted.

Please, review.

github-actions · 2022-09-01T19:31:17Z

Hey!

Congratulations on your PR! 😎😎😎

Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code.

Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate.

If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat.

When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly.

Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒

Please, make sure you haven't made common mistakes

Universal recommendations:

Give variables and functions meaningful names. Avoid generic names like item, element, key, object, array or their variations. Exception: helper functions that are specifically and intentionally designed to be multipurpose.
Function names should start with a verb as they denote actions; variables are normally nouns; boolean variables/functions start with is, does, has etc; variable containing multiple entities and functions returning lists contain entity name in plural form.
Have consistent code style and formatting. Employ Prettier to do all dirty work for you.
Use common sense or seek for an advice whenever requirements look ambiguous or unclear.

Also take a note of the requirements above and follow them in all your future projects.

By the way, you may proceed to the next task before this one is reviewed and merged.

Sincerely yours,
Submissions Kottachecker 😺

lysenko-sergey-developer · 2022-09-13T16:50:25Z

@misshatsker you should add a comment that you finish your self-check before a mentor starts to review your PR.

misshatsker · 2022-09-13T16:54:12Z

@misshatsker you should add a comment that you finish your self-check before a mentor starts to review your PR.

@lysenko-sergey-developer thank you for informing!) I will do it immediately.

misshatsker · 2022-09-13T16:55:29Z

Hey!

Congratulations on your PR! 😎😎😎

Let's do some self-checks to fix most common issues and to make some improvements to the code before reviewers put their hands on the code.

Go through the requirements/most common mistakes listed/linked below and fix the code as appropriate.

If you have any questions to requirements/common mistakes feel free asking them here or in Students' chat.

When you genuinely believe you are done put a comment stating that you have completed self-checks and fixed code accordingly.

Also, be aware, that if you would silently ignore this recommendation, a mentor can think that you are still working on fixes. And your PR will not be reviewed. 😒

Please, make sure you haven't made common mistakes

Universal recommendations:

Give variables and functions meaningful names. Avoid generic names like item, element, key, object, array or their variations. Exception: helper functions that are specifically and intentionally designed to be multipurpose.

Function names should start with a verb as they denote actions; variables are normally nouns; boolean variables/functions start with is, does, has etc; variable containing multiple entities and functions returning lists contain entity name in plural form.

Have consistent code style and formatting. Employ Prettier to do all dirty work for you.

Use common sense or seek for an advice whenever requirements look ambiguous or unclear.

Also take a note of the requirements above and follow them in all your future projects.

By the way, you may proceed to the next task before this one is reviewed and merged.

Sincerely yours, Submissions Kottachecker 😺

I have finished self-check

lysenko-sergey-developer · 2022-09-13T21:52:38Z

@misshatsker Great work! 💯

lysenko-sergey-developer · 2022-09-13T21:21:30Z

submissions/misshatsker/document-object-model/index.css

+.header .menu-item .link:hover,
+.header .menu-btn:hover,
+.active-menu-item .link {
+  background-color: #f4f4f4;


Move this colour also to :root

lysenko-sergey-developer · 2022-09-13T21:25:10Z

submissions/misshatsker/document-object-model/index.html

+    </header>
+    <main>
+        <div class="full-main-page">
+            <img class="media-animal" alt="">


Try to fill an alt attribute with an image description. That increases your code accessibility.

lysenko-sergey-developer · 2022-09-13T21:28:28Z

submissions/misshatsker/document-object-model/index.js

+        title: "Cats",
+        media: "./media/cat.jpg",
+        description: "Domestic cats are natural hunters able to stalk prey and pounce with sharp claws and teeth. They are particularly effective at night, when their light-reflecting eyes allow them to see better than much of their prey. Cats also enjoy acute hearing. All cats are nimble and agile, and their long tails aid their outstanding balance.",
+        propertyLifespan: "Up to 18 years",


property prefix is redundant

lysenko-sergey-developer · 2022-09-13T21:29:28Z

submissions/misshatsker/document-object-model/index.html

+                <div class="animal-main-info">
+                    <h1 class="animal-category-title"></h1>
+                    <p class="animal-category-description"></p>
+                    <ul class="animal-property-list" >


Redundant space. Configure your code editor to see redundant spaces/tabs.

lysenko-sergey-developer · 2022-09-13T21:32:16Z

submissions/misshatsker/document-object-model/index.js

+    }
+];
+
+const menuIcon  = document.getElementsByClassName("menu-icon")[0];


Using document.querySelector will be more readable.

lysenko-sergey-developer · 2022-09-13T21:38:42Z

submissions/misshatsker/document-object-model/index.js

+
+let isOpened = false;
+menuIcon.onclick = () => {
+    if (isOpened) {


You can skip additional var isOpened if you will use menuIcon.classList.contains

lysenko-sergey-developer · 2022-09-13T21:39:51Z

submissions/misshatsker/document-object-model/index.js

+        activeLi = li;
+    }
+
+    li.innerHTML = '<a class="link" href="#'+ animalTitle + '">' + animalTitle + '</a>';


Try to use template string over concatenations by +

lysenko-sergey-developer · 2022-09-13T21:44:50Z

submissions/misshatsker/document-object-model/index.js

+
+changeMainPageInfo(animals[0]);
+
+let activeLi = null;


Try to wrap this part of the code in function with meaning name. Also will be cool if you will store activeLi in closure.

lysenko-sergey-developer · 2022-09-13T21:45:58Z

submissions/misshatsker/document-object-model/index.js

+    li.innerHTML = '<a class="link" href="#'+ animalTitle + '">' + animalTitle + '</a>';
+
+    li.onclick = () => {
+        closeMenu();


It closes menu each time, and even menu is not displayed.

lysenko-sergey-developer · 2022-09-13T21:48:46Z

submissions/misshatsker/document-object-model/index.js

+
+    li.innerHTML = '<a class="link" href="#'+ animalTitle + '">' + animalTitle + '</a>';
+
+    li.onclick = () => {


Try to use event bubbling technique to avoid adding a listener on each list item.
https://blog.logrocket.com/deep-dive-into-event-bubbling-and-capturing/

lysenko-sergey-developer · 2022-09-13T22:00:07Z

UI/UX Improvement:

Main content should have left and right margins between 1080px and 768px
Image squeezing at screen width 800px. Your images have different aspect ratios. Try to play with them.

misshatsker · 2022-09-16T17:02:50Z

@lysenko-sergey-developer Thanks a lot for your review, Could you please check the changes?

lysenko-sergey-developer · 2022-09-16T18:13:07Z

submissions/misshatsker/document-object-model/index.html

+                    <p class="animal-category-description"></p>
+                    <ul class="animal-property-list">
+                        <li class="property">
+                           <img class="property-icon" src="./icons/heart.png" alt="">


Fill all empty alts.

lysenko-sergey-developer · 2022-09-16T18:14:30Z

submissions/misshatsker/document-object-model/index.js

+function appendMenuItems() {
+    let previousLiEl = null;
+
+    for (let i = 0; i < animals.length; i++) {


Change for to forEach

And move to function with a name something like fill Menu

lysenko-sergey-developer · 2022-09-16T18:20:16Z

submissions/misshatsker/document-object-model/index.js

+    weight.textContent = animal.weight;
+}
+
+changeMainPageInfo(animals[0]);


Move initial app state calls to file bottom

lysenko-sergey-developer · 2022-09-16T18:29:20Z

@misshatsker Good job! We need one last push and we did it! 🥇

misshatsker · 2022-09-16T18:42:20Z

@lysenko-sergey-developer Thanks! Could you re-review plz?)

lysenko-sergey-developer

LGTM! 😃

Add DOM homework

8705f19

madmaxWMFU added the task-DOM label Sep 1, 2022

Add logic optimizations

3e469c9

lysenko-sergey-developer requested changes Sep 13, 2022

View reviewed changes

lysenko-sergey-developer self-assigned this Sep 15, 2022

Fixes

29f9360

misshatsker requested a review from lysenko-sergey-developer September 16, 2022 17:02

lysenko-sergey-developer requested changes Sep 16, 2022

View reviewed changes

Last push ;)

e033d97

misshatsker requested a review from lysenko-sergey-developer September 16, 2022 18:41

lysenko-sergey-developer approved these changes Sep 16, 2022

View reviewed changes

lysenko-sergey-developer merged commit c8ddd38 into kottans:main Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DOM homework #392

Add DOM homework #392

misshatsker commented Sep 1, 2022

github-actions bot commented Sep 1, 2022

lysenko-sergey-developer commented Sep 13, 2022

misshatsker commented Sep 13, 2022

misshatsker commented Sep 13, 2022

lysenko-sergey-developer commented Sep 13, 2022 •

edited

Loading

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer Sep 13, 2022

lysenko-sergey-developer commented Sep 13, 2022

misshatsker commented Sep 16, 2022

lysenko-sergey-developer Sep 16, 2022

lysenko-sergey-developer Sep 16, 2022

lysenko-sergey-developer Sep 16, 2022

lysenko-sergey-developer Sep 16, 2022

lysenko-sergey-developer commented Sep 16, 2022

misshatsker commented Sep 16, 2022

lysenko-sergey-developer left a comment


		li.innerHTML = '<a class="link" href="#'+ animalTitle + '">' + animalTitle + '</a>';

		li.onclick = () => {

Add DOM homework #392

Add DOM homework #392

Conversation

misshatsker commented Sep 1, 2022

DOM

github-actions bot commented Sep 1, 2022

lysenko-sergey-developer commented Sep 13, 2022

misshatsker commented Sep 13, 2022

misshatsker commented Sep 13, 2022

lysenko-sergey-developer commented Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lysenko-sergey-developer commented Sep 13, 2022

misshatsker commented Sep 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lysenko-sergey-developer commented Sep 16, 2022

misshatsker commented Sep 16, 2022

lysenko-sergey-developer left a comment

Choose a reason for hiding this comment

lysenko-sergey-developer commented Sep 13, 2022 •

edited

Loading