Skip to content

feat: DOM Elements practise#154

Open
zanetasochon wants to merge 10 commits intodevmentor-pl:masterfrom
zanetasochon:master
Open

feat: DOM Elements practise#154
zanetasochon wants to merge 10 commits intodevmentor-pl:masterfrom
zanetasochon:master

Conversation

@zanetasochon
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown
Owner

@devmentor-pl devmentor-pl left a comment

Choose a reason for hiding this comment

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

Żaneto,

Dobra robota 👍
Zostało parę drobnych tematów do refaktoryzacji :)

01/app.js Outdated
@@ -1 +1,18 @@
console.log('DOM'); No newline at end of file
const commentsItemNewest = document.querySelector('.comments__item', '.comments__item--newest');
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Czy faktycznie został wyszukany zadany element czy po prostu pierwszy o klasie .comments__item? (efekt ten sam, ale treść zadania była inna)

Tutaj selektor powinien wyglądać tak: querySelector('.comments__item.comments__item--newest'); - teraz zadziała jak należy :)

01/app.js Outdated
@@ -1 +1,18 @@
console.log('DOM'); No newline at end of file
const commentsItemNewest = document.querySelector('.comments__item', '.comments__item--newest');
const childrenOfLi = commentsItemNewest.querySelectorAll('p');
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

TUtaj już mamy obiekt "tablicopodobny" czyli mamy dostęp do length więc zamiast pętli wystarczy po prostu: childrenOfLi.length

02/app.js Outdated
@@ -1 +1,11 @@
console.log('DOM'); No newline at end of file
const aList = document.querySelectorAll('a');
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Można też od razu wyszukać odpowiednie przez querySelectorAll('a[data-url]');

03/app.js Outdated
const button = document.createElement('button');

for (const key in buttonSettings) {
if (key === 'attr') {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Jeśli robimy if-y na każdy przypadek to nie będzie czytelniej zrobić to oddzielnie? tj.

for (const key in buttonSettings.attr) {
      if (key === 'className') {
        button.classList.add(key);
      } else {
        button.setAttribute(key, buttonSettings.attr[key]);
      }
    }
    
 for (const key in buttonSettings.css) {
      if (key === 'border') {
        button.style[key] = buttonSettings.css[key];
      } else if (key === 'padding') {
        button.style[key] = buttonSettings.css[key];
      } else if (key === 'color') {
        button.style[key] = buttonSettings.css[key];
      }
    }
    
    button.textContent = buttonSettings.text
    

Teraz widać, żę dla css można nawet prościej:

 for (const key in buttonSettings.css) {

       button.style[key] = buttonSettings.css[key];

}

05/app.js Outdated
Comment on lines +26 to +35
let siblingEl = curr.nextElementSibling;
const siblingsArr = [];

while ((curr = curr.nextElementSibling)) {
siblingsArr.push(siblingEl);
}

siblingsArr.forEach((el) => {
el.className = '.siblings';
});
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Co jeśli curr jest ostatnim dzieckiem? Czy to zadziała? Wygodniej jest zrobić curr.parentElement.children i wykluczyć curr przez if i mamy rozdzieństwo :)


addAEl.setAttribute('href', item.url);
addAEl.textContent = item.text;
});
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
Owner

@devmentor-pl devmentor-pl left a comment

Choose a reason for hiding this comment

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

Żaneto,

Teraz jest idealnie! :)

console.log(
'Zostało znalezionych elementów posiadających atrybut data-info w liczbie:',
spanArr.length
childrenOfP.length
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

link.setAttribute('href', hasAttribute);
}
hasAttribute = link.getAttribute('data-url');
link.setAttribute('href', hasAttribute);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

button.style[key] = buttonSettings.css[key];
}

button.textContent = buttonSettings.text;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

[...siblingEl].forEach((el) => {
if (el !== curr) {
el.className = 'siblings';
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

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