Skip to content

Quite a challenge!#19

Open
joelamb wants to merge 30 commits into
constructorlabs:masterfrom
joelamb:master
Open

Quite a challenge!#19
joelamb wants to merge 30 commits into
constructorlabs:masterfrom
joelamb:master

Conversation

@joelamb
Copy link
Copy Markdown

@joelamb joelamb commented Sep 23, 2018

localStorage proved very challenging, more time spent planning the structure of the app would have made development faster, but still a really enjoyable project. Just wish I could have got more done.

Comment thread src/index.js
searchFilmBytitle(params.query);
currentPageNum.textContent = params.pageNum;
document.querySelector('#film-details').innerHTML = '';
e.stopPropagation();
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.

is stopPropagation needed?

Comment thread src/index.js
const prevPage = document.querySelector('#page-nav .prev');
prevPage.addEventListener('click', e => {
e.preventDefault();
params.pageNum > 1 ? params.pageNum-- : false; // Prevent previous page returning negative page number
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.

if might work better here since we do not need the false

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved: if() substituted for ternary.

Comment thread src/index.js

/* infinite scroll */
const infiniteScroll = () => {
const trigger = document.querySelector('#');
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.

what does this do?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved. It's a TODO feature to add infinite scroll to search results

@dmitrigrabov
Copy link
Copy Markdown
Collaborator

Good work

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