publiclab / Leaflet.DistortableImage

A Leaflet extension to distort or "rubber sheet" images
https://publiclab.github.io/Leaflet.DistortableImage/examples/
BSD 2-Clause "Simplified" License
265 stars 284 forks source link

load image in sidebar as thumbnail #1319

Closed 7malikk closed 1 year ago

7malikk commented 1 year ago

Fixes #1295

UI changes when the images fetched are above 100

thumbnail1 0

7malikk commented 1 year ago

@jywarren What are your thoughts on this implementation of the image render in the sidebar? This is preceding the client-side pagination we discussed here: https://github.com/publiclab/Leaflet.DistortableImage/issues/1295#issuecomment-13536976300 I would take up that project in a separate issue and PR

7malikk commented 1 year ago

Hello @jywarren here is a draft for the server side pagination #1300 With @segun-codes's help (thanks again @segun-codes), I was able to achieve a Gmail look-alike pagination as requested

For a link with images over 100

pagination

For a link with images below 100

pagination-2

So upon your review of this PR - #1319, this would be integrated.

Thank you!

jywarren commented 1 year ago

Wow, this is quite impressive, i really like the filtering of thumbnails into thumbs especially, and the pagination looks great.

I have two requests -- first, we are working with some much bigger blocks of code here; can you use a couple comments to explain what each portion does? For example, line 117-141 is so dense -- what does each section do? How can we make this code more readable and maintainable?

The second is -- are there any ways we can reduce the number of lines here by creating re-usable functions? Can we get clever with how the HTML is generated to make it easier to reuse across the 3 different renderers, since that seems to be the largest source of new lines of code?

Thanks a lot for this complex and carefully written PR -- I'm hoping we can refine it a little bit before we move forward!

7malikk commented 1 year ago

@jywarren Thanks a lot for your review, as per your requests, of course, I will add the comments and break them into chunks of reusable code. Thanks again

7malikk commented 1 year ago

@jywarren I've added the comments and made the adjustments to the code for readability and maintainability, I think you'd like it As soon as this is merged I'd get the pagination in there, taking into account the corrections you made in this PR. Thank you so much, I am learning so much from working with you and the rest of the team.

7malikk commented 1 year ago

Hi @7malikk -- I've made some pretty big suggestions here, and I hope they make sense. Code style is not the kind of thing where there's a "right" answer, and everybody does things a little differently. But what I tried to explain is how I like to use comments to explain mysterious things, but to keep comments as short as possible by using descriptive variable and function names. And the structure of the code itself (not too many nested functions, similar named functions for related functionality) can help to make things readable.

This is looking really good. Let me know what you think about my review and suggestions. Thanks a lot for sticking with this one, we're almost there!

@jywarren Thanks a lot for the review and the suggestions, they make a lot of sense as it helps demystify the code. I would make the necessary changes and consult your expertise, afterward. Thanks again! 🙌🏾

7malikk commented 1 year ago

@jywarren made a refactor, and ended with a little chunky code but it seems readable and understandable, let me know what you think.

While refactoring I realized in other for the logic to fail gracefully into full-resolution images, I needed to filter initially, before the separate function calls.