Skip to content

uradjena prva tri zadatka, zadatak DOMTabela ne radi#1

Open
djpavle wants to merge 3 commits intomasterfrom
domaci1407
Open

uradjena prva tri zadatka, zadatak DOMTabela ne radi#1
djpavle wants to merge 3 commits intomasterfrom
domaci1407

Conversation

@djpavle
Copy link
Copy Markdown
Contributor

@djpavle djpavle commented Jul 14, 2020

Da li neko vidi zbog cega ne radi ova DOM tabela? Ja ne mogu da provalim sta je.

</div>
<script src="/src/js/DOMTabela.js"></script>
</body>
</html> No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Da li su ti podešeni linter i editorconfig?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Podeseni su, i svaki put uradim npm run lint:js:fix pre nego sto sacuvam. Ali vidim da i dalje neke stvari ne resava (tipa novi red na kraju itd) . Pretpostavljam da nesto ne radim kako treba

Comment on lines +9 to +12
<label for="redovi">Unesi broj redova</label>
<select name="redovi" id="redovi"></select><br>
<label for="kolone">Unesi broj kolona</label>
<select name="kolone" id="kolone"></select><br>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bolje je kad koristiš p ili div za grupisanje labela i elementa forme zajedno nego da stavljaš br.

<title>Detektor Ćirilice</title>
</head>
<body>
<form>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Forma koja ne može da se pošalje?

<button id="tasterZaValidaciju">Izvrši proveru pisma</button>

<div id="validacija"></div>
<script src="/src/js/detektorCirilice.js"></script>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bravo za putanje! Bilo bi fino da pokrećeš server tako da src folder bude root pa da putanje budu bez /src na početku.

const kolone = document.getElementById('kolone')
const dugme = document.getElementById('dugme')

for (let i = 0; i < 21 + 1; 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.

i < 21 + 1 je isto što i i <= 21

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Veruj mi da ni sam ne znam sta sam hteo sa ovim 21+1 ..

tabela += '</tr>'
}
tabela += '</table>'
domTabela.innerHTML = tabela
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Kad funkcija može da se napiše tako da prihvata parametre i sa return vraća vrednost a ne radi ništa usput, to je bolje nego kad se kombinuje to dvoje.
domTabela.innerHTML = tabela je "side effect" i ne bi trebal da stoji ovde. Ova funkcija i generiše tabelu i ispisuje je u html, kad već može neka radi samo jedno od ta 2.

Comment on lines +8 to +20
function validirajCirilicu () {
for (let i of poljeTeksta.value.split("")) {
if (cirilica.includes(i)) {
validacija.innerHTML = 'Tekst je napisan u ćirilici'
break
} else if (latinica.includes(i)) {
validacija.innerHTML = 'Tekst je napisan u latinici'
} else {
validacija.innerHTML = 'Piši srpski da te ceo svet razume'
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ovo bi trebalo da se preradi da vraća vrednost sa return.
Šta radi break u 12 liniji i zašto postoji u tom bloku a u bloku sa latinicom ne?

const listaGodina = document.getElementById('listaGodina')
function generisiGodine (pocetna, zavrsna) {
for (let i = pocetna; i < zavrsna + 1; i++) {
listaGodina.innerHTML += `<option>${i}</option>`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Probaj da ga preradiš da koristi DOM metode, createElement i appendChild.
Takođe probaj da vraćaš vrednost iz funkcije sa return.

}
for (let i = 0; i < 7; i++) {
poljeZaIspisBrojeva.innerHTML += brojevi.pop() + '<br>'
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Preradi funkciju da koristi return.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Napravio sam novu komit sa svim izmenama. Da li treba da napravim i novi pull request za izmenjenu verziju?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ne to je to. Svi komiti idu na isti pull request dok ga ne sredimo.

function generisiBrojeve () {
poljeZaIspisBrojeva.innerHTML = ' '
const brojevi = []
for (let i = 1; i < 37; 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.

od do vrednosti bi trebalo da idu u parametre pa ćeš funkciju moći da iskoristiš na više mesta.
<= je često bolji izbor od < ako ti je bitno da se na prvo čitanje razume koji broj je gornja granica.

@frontend-comtrade
Copy link
Copy Markdown

Da li neko vidi zbog cega ne radi ova DOM tabela? Ja ne mogu da provalim sta je.

Pretpostavljam da si sredio u međuvremenu. Funkcija radi.

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