sidebar pagination

[7malikk]

Fixes #1300

• [x] PR is descriptively titled 📑 and links the original issue above 🔗
• [x] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt test
• [x] code is in a uniquely-named feature branch and has no merge conflicts 📁
• [x] screenshots/GIFs are attached 📎 in case of UI updates
• [x] @mention the original creator of the issue in a comment below for help or for a review

For a link with images over 100

For a link with images below 100

[7malikk]

@jywarren Hello, here's a PR for the pagination, its a tad chunky, implemented some of the lessons from the last PR. I do look forward to your review, thank you!

[7malikk]

Hello, @jywarren @PeculiarE here is my implementation of the modules. Thank you so much @PeculiarE for your help.

I decided to go with a class as implied in our discussion, it served its purpose satisfactorily in my opinion, but I'd love to hear your thoughts on it. Thank you.

I also added some comments, but I believe it's quite understandable at a glance, I am open to suggestions.

@segun-codes I could use your opinion too, thank you!

[7malikk]

For readability purposes, do you want to change class name from Paginate to Paginator ? If yes, you may also want to rename the file pagination.js to paginator.js. Finally, can prevBtn, nextBtn and range be initialized in the class constructor, without breaking anything, just to have complete encapsulation?

Great work so far @7malikk!

Great suggestions @segun-codes Concerning the paginate to Paginator that sounds great! I'd give all you've said a try and see how it plays out.

Thanks so much for your input.

[7malikk]

Hi! I made some further suggestions for how we can tuck even more functionality into the Pagination class, and not have to have it interact with outside code at all. I think this really simplifies the main archive.js file -- tell me what you think, and if you'd like I can suggest some ways to implement this!

Thank you so much @jywarren I think they're very good suggestions And it's a good objective to simplify the main archive.js. I would give all you've said a try, if I do come into any issues, I'd reach out to you for some suggestions. If that's fine by you? @jywarren Thank you once again.

[7malikk]

@jywarren I am currently working on an implementation that could move a lot of code to the Pagination Class code such as the renderImages and renderThumbnails functions, alongside the handleNext and handlePrev functions.

What do you think?

[jywarren]

@jywarren I am currently working on an implementation that could move a lot of code to the Pagination Class code such as the renderImages and renderThumbnails functions, alongside the handleNext and handlePrev functions.

What do you think?

Hi @7malikk - I think it makes sense to me that renderImages and renderThumbnails remain in archive.js, as they are not specific to pagination. However, at some point, I could imagine breaking them out into an archive-interface.js file, or maybe /examples/js/archive/interface.js? - so that archive.js begins to be where all the code comes together at a high level for integration, but implementation of each specific kind of code, or each different functionality, is broken into other files.

I think a lot about how best to break things up, and one way to do it is to think - can this code work, fundamentally, if you remove this module? In this case - is pagination core to the functionality? It could be said that it's peripheral - makes the experience better, but isn't critical. That's a good candidate for breaking out. But rendering images, is that core? More so. It's not the only way to subdivide code, but it's helpful.

[7malikk]

@jywarren I am currently working on an implementation that could move a lot of code to the Pagination Class code such as the renderImages and renderThumbnails functions, alongside the handleNext and handlePrev functions. What do you think?

Hi @7malikk - I think it makes sense to me that renderImages and renderThumbnails remain in archive.js, as they are not specific to pagination. However, at some point, I could imagine breaking them out into an archive-interface.js file, or maybe /examples/js/archive/interface.js? - so that archive.js begins to be where all the code comes together at a high level for integration, but implementation of each specific kind of code, or each different functionality, is broken into other files.

I think a lot about how best to break things up, and one way to do it is to think - can this code work, fundamentally, if you remove this module? In this case - is pagination core to the functionality? It could be said that it's peripheral - makes the experience better, but isn't critical. That's a good candidate for breaking out. But rendering images, is that core? More so. It's not the only way to subdivide code, but it's helpful.

I never thought about it in such a 'grand-scheme of things' way, and i totally agree as to breaking files where the functionality broken off is not core to the program. Thank you for your insight as usual @jywarren I am really glad to be your mentee.

[7malikk]

Hello, @jywarren I think this is ready for a merge. let me know what you think, thank you!

[7malikk]

This looks fantastic and I think it is ready to merge - with one last question first... where is range defined? Did some code get deleted by mistake? Just checking! Thanks!

Oh not at all It's a js shorthand, where one doesn't need to define a variable as long as the id value in the Html and the reference in the js file are the same. for example:

In the .html file
<p id='range'> My Range </p>

In the .js file
console.log(range.innerHtml) // <--- this will return "My Range"

I could define it to avoid any misconceptions.

@jywarren

[jywarren]

Oh not at all It's a js shorthand, where one doesn't need to define a variable as long as the id value in the Html and the reference in the js file are the same. for example:

I could define it to avoid any misconceptions.

OMG i read about that! Let's leave it -- very nice.

If you could resolve the conflicts by rebasing, I'll merge. Thanks @7malikk !!!

[7malikk]

Alright @jywarren

[7malikk]

@jywarren Final touches have been made, can we proceed? Thank you! 🎉

[jywarren]

Hooray! Great work!!!!

[7malikk]

Hooray! Great work!!!!

Thank you @jywarren!!

[segun-codes]

Hooray! Great work!!!!

Thank you @jywarren!!

Congratulations @7malikk

[7malikk]

Congratulations @7malikk

Thank you @segun-codes😌😌