Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Friends app #95

Merged
merged 9 commits into from
Sep 8, 2022
Merged

Friends app #95

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24f2253
js core code
iiziumov Aug 8, 2022
055be7a
added friends-app
iiziumov Aug 8, 2022
64e1e2e
remove unnecessary file from friends app directory
AsaMitaka Aug 19, 2022
1a3f727
fixed func, fixed css, fixed variable names, func names
AsaMitaka Aug 20, 2022
23ab608
fixed search func, fixed findByAge func
AsaMitaka Aug 20, 2022
b261928
fix func findByAge
AsaMitaka Aug 22, 2022
2bf8877
rewrite functions, fetch, fixex css styles, fixed html
AsaMitaka Aug 29, 2022
466171d
rewrite render logic, add filterState, rewrite filterBy and sortBy an…
AsaMitaka Sep 4, 2022
f1b6488
rewrite function names, rewrite switch case in filterByGender func
AsaMitaka Sep 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions submissions/asaMitaka/FriendsApp/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Friends App</title>
<link rel="stylesheet" href="style.css">
</head>
<body>
<div class="container">
<header>
<a href="https://kottans.org/" target="_blank">Kottans</a>
</header>

<main class="mainBlock">
<aside class="asideBlock">
<form name="age" class="asideBlock__items">
<p>By Age</p>
<input type="radio" name="age" id="ageMore" value="ageMore" class="age">
<label for="ageMore">Age < </label>
<input type="radio" name="age" id="ageLess" value="ageLess" class="age">
<label for="ageLess">Age > </label>
</form>

<form class="asideBlock__items" name="last">
<p>By Last Name</p>
<input type="radio" name="last" id="lastAToZ" value="lastAToZ" class="last">
<label for="lastAToZ">A-Z</label>
<input type="radio" name="last" id="lastZToA" value="lastZToA" class="last">
<label for="lastZToA">Z-A</label>
</form>

<form class="asideBlock__items" name="minMax">
<p>Age 10 - 100</p>
Copy link

Choose a reason for hiding this comment

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

Why is it exactly 10 - 100?

<input type="number" name="minNumber" id="minNumber" min="10" value="10">
<input type="number" name="maxNumber" id="maxNumber" max="100" value="100">
</form>

<form class="asideBlock__items" name="search">
<input type="text" id="searchBar" name="search" required
minlength="1" maxlength="10" size="20" placeholder="search">
</form>

<form class="asideBlock__items" name="gender">
<input type="radio" id="male"
name="gender" value="male" class="gender">
<label for="male" class="gender">MALE</label>

<input type="radio" id="female"
name="gender" value="female" class="gender">
<label for="female">FEMALE</label>

<input type="radio" id="all"
name="gender" value="all" class="gender" checked>
<label for="all">All</label>
</form>

<form class="asideBlock__items" name="reset">
<button class="reset" name="reset" data-attr="reset">RESET</button>
</form>
</aside>
<article class="articleBlock"></article>
</main>
<footer>
<a href="https://kottans.org/" target="_blank">Kottans</a>
</footer>
</div>
<script src="script.js"></script>
</body>
</html>
237 changes: 237 additions & 0 deletions submissions/asaMitaka/FriendsApp/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
const url = 'https://randomuser.me/api/?results=12';
const form = document.forms;
Copy link

Choose a reason for hiding this comment

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

Why do you have 2 forms? You should use 1 form and get it by id or something, don't rely on document.forms.

const content = document.querySelector('.articleBlock');

let friendsCopy = [];
let friends = [];
let isSearch = false;

async function getResponse(url) {
let response;
Copy link

Choose a reason for hiding this comment

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

You don't need an outside variable for response. Do the .json() thing right in the try block. You have a faulty logic here, you're getting data from a response outside of try in any scenario, even if the data isn't there because you ignore the catch block. The only reason your program doesn't crash is that if there is an error you just reset your page, but let's not rely on that. If there is no data - don't extract it from the response.

try {
response = await fetch(url);
if (!response.ok) {
throw new Error(response.statusText);
}
} catch (err) {
console.log(err);
resetPage();
}
let jsonData = await response.json();
Copy link

Choose a reason for hiding this comment

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

const?


friendsCopy = jsonData.results;
friends = jsonData.results;

renderAllItemsToPage(friends);
}

function creatingDivItem(item) {
Copy link

Choose a reason for hiding this comment

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

What div item? What is the content of this item? Use more descriptive names, please.

return `
<div class='content__item'>
<img class='content__item-img' src='${item.picture.medium}'>
<div class='content__item-name'>${item.name.title} ${item.name.first} ${item.name.last}</div>
<p class='content__item-email'>
<a href="mailto: ${item.email}" target="_blank"> ${item.email}</a>
</p>
<p>Age: ${item.dob.age}</p>
<a href='${item.phone}'>${item.phone}</a>
<p class='content__item-country'>${item.location.country}, ${item.location.city}</p>
<div class='content__item-gender'>${item.gender}</div>
</div>
`;
}

form.last.addEventListener('click', function(e) {
Copy link

Choose a reason for hiding this comment

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

Don't rely on the fact that your form is the last one on the page. Reason: there are a lot of browser extensions that modify the dom, for example, Grammarly adds its script to the end of DOM to work. That is why we don't style by tags and don't do JS by tags - because you never know what could go wrong with the DOM structure. Give your form an ID and do a querlySelector.

if (e.target && e.target.classList.contains('last')) {
checkLastSort(e);
Copy link

Choose a reason for hiding this comment

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

You only need target from event object, you can destructure it on the start of the function like

function({ target }) { ...

}
});

function checkLastSort(event) {
form.age.reset();
switch(event.target.value) {
case "lastAToZ":
sortedAtoZLast();
break;
case "lastZToA":
sortedZtoALast();
break;
}
}

form.reset.reset.addEventListener('click', resetPage);
Copy link

Choose a reason for hiding this comment

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

Consider resetting the form state and rerendering users instead of reloading the whole page. You can also use this.


form.age.addEventListener('click', function(e) {
Copy link

Choose a reason for hiding this comment

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

You add a lot of event listeners for separate items of one form. Please combine them all into one event listener for the whole form. If you use the correct event type, this event listener will fire each time you press a button in your search input and each time a radiobutton is pressed.

if (e.target && e.target.classList.contains('age')) {
sortByAge(e);
}
});

function sortByAge(event) {
Copy link

Choose a reason for hiding this comment

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

Same thing with target.

form.last.reset();
switch(event.target.value) {
case "ageMore":
sortedAgeMore();
break;
case "ageLess":
sortedAgeLess();
break;
}
}

function sortedAgeMore() {
Copy link

Choose a reason for hiding this comment

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

Why sorted? It's not a verb. You had sort in your other function.

friends.sort((a, b) => a.dob.age - b.dob.age);
renderAllItemsToPage(friends);
}

function sortedAgeLess() {
friends.sort((a, b) => b.dob.age - a.dob.age);
renderAllItemsToPage(friends);
}

function sortedAtoZLast() {
friends.sort((a, b) => a.name.last !== b.name.last ? a.name.last < b.name.last ? -1 : 1 : 0);
renderAllItemsToPage(friends);
}

function sortedZtoALast() {
friends.sort((a, b) => a.name.last !== b.name.last ? a.name.last > b.name.last ? -1 : 1 : 0);
renderAllItemsToPage(friends);
}

form.gender.addEventListener('click', function(e) {
if (e.target && e.target.classList.contains('gender')) {
checkedGender(e);
}
});

function checkedGender(event) {
switch (event.target.value) {
case 'male':
Copy link

Choose a reason for hiding this comment

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

This is filtering, it should not belong to the sorting function.

renderMale();
break;
case 'female':
renderFemale();
break;
case 'all':
renderAll();
break;
}
}

function renderMale() {
Copy link

Choose a reason for hiding this comment

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

Your renderMale, renderFemale and renderAll functions are very similar. Please combine them.

friends = friendsCopy.slice();
friends = friends.filter(item=> item.gender === 'male');
Copy link

Choose a reason for hiding this comment

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

We usually place a space around => in arrow functions. So item => ...

renderAllItemsToPage(friends);
}

function renderFemale() {
friends = friendsCopy.slice();
Copy link

Choose a reason for hiding this comment

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

.slice() without arguments just returns the same array, why is it here?

friends = friends.filter(item=> item.gender === 'female');
renderAllItemsToPage(friends);
}

function renderAll() {
friends = friendsCopy.slice();
renderAllItemsToPage(friends);
}

form.minMax.addEventListener('change', checkNumber);
form.minMax.addEventListener('change', findByAge);

function checkNumber() {
if (minNumber.value < 10) {
minNumber.value = 10
return;
}

if (maxNumber.value > 100 && minNumber.value < 10) {
maxNumber.value = 100;
minNumber.value = 10;
return;
}

if (maxNumber.value > 100) {
maxNumber.value = 100;
return;
}

findByAge();
}

function findByAge() {
Copy link

Choose a reason for hiding this comment

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

In terms of JS, find means 1 result and filter means multiple results.

friends = friendsCopy.slice();

if (form.age.ageLess.checked) {
sortedAgeLess();
}

if (form.age.ageMore.checked) {
sortedAgeMore();
}

if (form.last.lastAToZ.checked) {
sortedAtoZLast();
}

if (form.last.lastZToA.checked) {
sortedZtoALast();
}

friends = friends.filter(item => {
if (item.dob.age > minNumber.value && item.dob.age <= maxNumber.value) {
return item;
}
});

renderAllItemsToPage(friends);
}

form.search.search.addEventListener('input', searchFunc);

function searchFunc() {
isSearch = true;
Copy link

Choose a reason for hiding this comment

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

You don't need a variable to track if there is something in the input. Call this function every time something changes, just don't do anything here if the input is empty.

let val = form.search.search.value.toLowerCase();
if (val && val.length > 0) {
content.innerHTML = '';
Copy link

Choose a reason for hiding this comment

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

Why is this here? It's a searching function, it should return an array of users that match search input, not do things in the DOM.


let acc = '';
friends.forEach(item=> {
if(item.name.first.toLowerCase().includes(val) || item.name.last.toLowerCase().includes(val)) {
acc += creatingDivItem(item);
}
});
content.innerHTML = acc;
} else {
isSearch = false;
renderAllItemsToPage(friends);
}
}

function resetPage() {
minNumber.value = 10;
maxNumber.value = 100;
friends = friendsCopy.slice();

renderAllItemsToPage(friends);
}

function renderAllItemsToPage(arr) {
if (form.search.search.value.length === 0) {
isSearch = false;
}

content.innerHTML = '';

let acc = '';
Copy link

Choose a reason for hiding this comment

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

Instead of creating a string of elements, it would be better to use the map method and create an array of elements, just join them before rendering. This would allow you to manipulate this array in the future if needed.

arr.forEach(el => {
acc += creatingDivItem(el);
});
content.innerHTML = acc;

if(isSearch) {
searchFunc();
}
}

getResponse(url);
Loading