Skip to content

Develop#2

Merged
jskiller1404 merged 23 commits into
masterfrom
develop
Dec 6, 2021
Merged

Develop#2
jskiller1404 merged 23 commits into
masterfrom
develop

Conversation

@jskiller1404

Copy link
Copy Markdown
Owner

Add serach input for projects, and add arays of projects and nav items.

@jskiller1404
jskiller1404 requested a review from IAmVisco December 4, 2021 16:35
Comment thread components/NavItems/NavItems.js Outdated
<ul class="menu-sublist">
`
submenu.forEach((item) => {
switch(item){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you really need switch case here? It's a rare construction that you can usually avoid.

</li>
`
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

idk what's wrong with your IDE but set up proper formatting. Getting comfortable with tools you use should be step 1. It saves a lot of headache later down the road.

Comment thread components/NavItems/NavItems.js Outdated
</ul>
</nav>
`
for (let i = 0; i < ROOT_NAV_ITEMS.length; i++) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Read about map, filter, reduce, forEach and other array methods. Using default for loop is highly discouraged.

Comment thread components/Projects/Projects.js Outdated
class Projects {
render() {
let htmlCatalog = "";
PROJECTS.forEach(({ id, imageUrl, title, text }) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why spacing is different in this file? Code style should be united across all files.

Comment thread script.js Outdated
);
}
}
if(isMobile.any()){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I doubt this should be done with a lot of copy pasted js code. CSS feels more appropriate.

Comment thread script.js Outdated
document.body.classList.add('_touch');
let menuArrows = document.querySelectorAll('.menu-arrow');
if(menuArrows.length > 0){
for(let i = 0; i < menuArrows.length; i++){

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Again, don't use default for loops.

Comment thread script.js
}

document.querySelector('.proj-input').oninput = function(){
setTimeout(() => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This whole fucntion is a huge mess. I'm sure there's a much cleaner way to do it.

@github-pages
github-pages Bot temporarily deployed to github-pages December 6, 2021 15:23 Inactive
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