openwisp / django-ipam

The development of this project has moved to openwisp-ipam
https://github.com/openwisp/openwisp-ipam
BSD 3-Clause "New" or "Revised" License
78 stars 28 forks source link

[ui] Add infinite pagination/scrolling in subnet's visual hosts view #66 #89

Closed pawelplsi closed 4 years ago

pawelplsi commented 4 years ago

Should fix #66

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 351


Totals Coverage Status
Change from base Build 345: 0.0%
Covered Lines: 567
Relevant Lines: 567

💛 - Coveralls
pawelplsi commented 4 years ago

I have implemented this feature the way wanted there: https://github.com/openwisp/django-ipam/issues/66#issuecomment-568049719, there are only 5 pages in DOM tree at time to reduce memory usage, however a few measurements have shown me that keeping these pages as a JS object doesn't really consume much memory compared to the entire page, so in this implementation they are stored this way client-side so there's no need to fetch one page from server multiple times. The only drawback of this implementation I've found is that when we remove pages from the beginning (to save memory), layout of lists items representing hosts may be shifted a bit (depending on window width), and since browser size may change, I don't have any idea to prevent such behavior, I've already tried to replace these list items with placeholders in order to keep the layout as it is, but it turned out not to be memory efficient enough.

I see visualization is disabled on IPV6 networks, should I make it work?

atb00ker commented 4 years ago

I created a really large subnet like 11.0.0.0/7 and the page is auto loading all the ?page=X, even without scrolling, does it happen on your end as well? :smile:

pawelplsi commented 4 years ago

@atb00ker No, it's woking with such subnet on my end, could you tell what browser do you use and how does it exactly behave? I have tested in both on firefox and chromium.

atb00ker commented 4 years ago

Hi, sorry for sharing a google drive link but I don't have the time to debug this so I am attaching a video, I am using Chrome:79.0.3945.88 Link : https://drive.google.com/file/d/1dZNxzvLDDGVda7G5ARHCSG5c2Xf-A20c/view?usp=sharing

I tested on Firefox 68.3.0esr, it worked fine.

atb00ker commented 4 years ago

The behaviour isn't smooth, it's hard to navigate from one subnet to another or to find a specific subnet.

@nemesisdesign what do you think?

I am thinking to peek into the implementation of phpipam and see their solution instead.

pawelplsi commented 4 years ago

I changed that implementation a bit, now start of every page is aligned to the left, it may look a bit worse, but with this approach it's no longer 'hopping' when scrolling. I also turned off browser's automatic scroll anchoring and replaced it with a code that manually prevents random scrolling, It should fix your issue on chrome @atb00ker, could you check if it did?

pawelplsi commented 4 years ago

@nemesisdesign Scrolling by mouse works, but you need to focus the scroll container first (at least on FF), and yes, it is a bit quirky but we cannot make it perfect when scrolling with mouse since new pages are added so the scroll bar is not fixed-size. For small subnets it looks as it used to, the only difference is that there is a scroll container, i can make it bigger, the scrollbar would not appear then in small subnets

nemesifier commented 4 years ago

@pawelplsi I mean I cannot scroll by pressing the mouse left button and dragging it up or down

pawelplsi commented 4 years ago

I mean I cannot scroll by pressing the mouse left button and dragging it up or down

Huh, the scrollbar was partially covered with right menu/drawer object, one of most annoying things I debugged recently :smile:

pawelplsi commented 4 years ago

I added IPv6 support, that let me simplify some logic also, and wrote a custom pagination (start adddress parameter instead of page number) that will be more convenient later in fixing #91, now I only need to write some more tests to increase the coverage. But I'm not sure about one thing, should that pagination and host set implementation be in generics.py? Do you think I should move it somewhere?

atb00ker commented 4 years ago

I just noticed that the spacing is not even in the last line of each page, it's not a blocking issue, but if it's easily fixable, please look into it... send

pawelplsi commented 4 years ago

@atb00ker added fixed width margins to these cells, it seems to be fixed now :smile:

pawelplsi commented 4 years ago

Thanks for your appreciation @nemesisdesign, but are you sure inlining page divs is the right way to go? I have not done it that way by an accident. The pages are not always as wide as your screen is, so after a page is removed from the beginning (to save memory) the pages are aligned differently, so the hosts are 'hopping' through the list, as I explained in comments above, I tried to solve it but I didn't manage to do it any efficient way, and I think it's possible there's no 'not-dirty' way to solve that. Although the line breaks may not look really well to you, I think that the problem above is a way more annoying.

nemesifier commented 4 years ago

@pawelplsi occasional hops while scrolling an infinite scroller is pretty common, but if you can find a solution that solves both problems (line break and hopping) would be great!

Otherwise we can live with the occasional hopping, the line-break is too obvious to keep, it makes it look unprofessional.