feat: infinity scroll via button in scenes modal

[kfarr]

https://github.com/3DStreet/3dstreet-editor/issues/304

[kfarr]

Hi @rostyslavvnahornyi , thanks for this first attempt and it was a helpful learning experience to understand the complexities of pagination. This PR has some issues I wanted to share, and it may be advise-able to retry this task with a new strategy.

There are 2 goals that I am evaluating this on:

• User interface - is this better for the user than before, or at least the same?
• Effectiveness - does this reduce bandwidth of query responses?

Against both goals this PR performs worse than prior: It delays the user by reloading data that has already been loaded, and it uses more bandwidth to retrieve scenes than before.

But the good news is I hopefully can help with a more clearly defined test case which I document below. I don't have an idea for a better strategy as pagination is complicated, so I wish you luck in finding a solution that works!

Here is the test case:

• test: open a new browser, open network tools, disable caching and start recording, open from localhost after running npm run start:dev in 3dstreet-editor repo, log-in, note current bandwidth (transfer) level in network tools, click open button, scroll to the bottom of my scenes, then click community tab, scroll to bottom of all the scenes. Note bandwidth (transfer) level again.
• I tested this PR against the current target branch epic-v2-save-load

TEST1 - current epic-v2-save-load branch

• After editor is loaded and logged-in, at 25MB transfer, 37.5MB resources before clicking open menu
• After clicking open menu, at 27MB transfer, 125MB resources after clicking open menu - (Delta of 2MB to see all items, 0MB for scrolling scenes or switching tabs after initial load)

TEST2 - this infinity scroll via button branch

• After editor is loaded and logged-in, at 25.7MB transfer, 70.4MB resources before clicking open menu (note increase of 0.7MB already)
• After clicking open menu, at 26.5MB transferred, 103MB resources after clicking open menu (note this is less than 27MB above by 300kb -- so we do have a temporary "win" of -300kb savings compared to other method assuming a user does not switch tabs or click more button!)
• click show more1 27.2MB transferred (at this point it uses more bandwidth than the other method)
• click show more2 27.9MB transferred -- now at bottom of my scenes
• now click community scenes
• 29.1MB transferred, 224MB resources
• after scrolling to the bottom of community scenes by pressing "more" 4 times
• 32.7MB transferred, 388MB resources - (Delta of 7MB to see all items, 1MB+ for each tab press or show more press after)
• Clicking My Scenes tab again results in 33.5MB
• Always transfers at least 1MB when clicking to other tab

specific usability issues:

• Right now as a user switching between community and "my scenes" tabs it "keeps my position" when I switch from one tab to another. So I can resume my browsing if I click back at another. However in this new method when switching tabs the previous position is lost and the entire tab is reloaded from the top. Instead it should act like current behavior where scroll position is kept when tab is returned to later.
• The "Show More" button sometimes will stop appearing after switching tabs more than twice and having pressed "Show More" at least once on each tab

[kfarr]

Hi @rostyslavvnahornyi I am not sure why, but this works OK when using dev .env config but it does not work for production .env config (even running the same local code). See here for a video showing the issue: https://github.com/3DStreet/3dstreet-editor/assets/470477/7c3e6a9e-c84d-4e80-8f96-363787f5d047

[rostyslavvnahornyi]

Hi, @kfarr !

The issue was with the production configuration, so I added a new webpack production config. To start the production environment, use the following commands:

npm run start:build
npm run start:prod

With this configuration, the problem with infinite pagination is resolved in the production environment.

[kfarr]

Here is an updated list of issues after this QA pass:

• [x] - npm install to add new dependencies: error, see below - KF fixed Nov 13
• [x] - npm run start:prod error "serve: command not found" - KF fixed Nov 13
• [ ] ⚠ core issue not resolved -- Each fetch to request records still appears to download all records from the entire database (~7MB of bandwidth) instead of paginating to just 20 records. Each press of a "Load More" button downloads the entire database at ~7MB. [see video1]
• [ ] - @rostyslavvnahornyi can we revert the change back to 'aframe-inspector-opened' instead of 'aframe-inspector-isOpened' css selector? This change may break core ui in core 3dstreet repo that assumes the old syntax.
• [ ] - @rostyslavvnahornyi why are there new js files in /dist dir; why new index.html file in /dist dir? is this part of 3DStreet/3dstreet#591 ? Is there any way to first only address the core infinity loader ticket and keep just to 1 file, and then add multi file later? It introduces deployment complexity and other unknowns
• [ ] When clicking "open" button without being logged in, the open browse modal shows a spinner and does nothing (console error: Uncaught (in promise) FirebaseError: Function where() called with invalid data. Unsupported field value: undefined)
• [ ] KF - to do deployment test on dev firebase server to see how multi file js works; try from a-frame inspector invocation on 3rd party a-frame scene on glitch or github
• [ ] new deployment mechanism results in file not found for some icons such as left or right arrows to collapse side panels
• [ ] commit with large diff and 2 words commit comment "minor fixes" isn't sufficient, please include at least a basic description of what has been done @rostyslavvnahornyi

Later tickets

• [ ] - after saving a new scene in editor, open scene browse doesn't display it. have to reload the application. (can ignore for now, make separate ticket)
• [ ] - after renaming in the open browse rename option, clicking scene does not load the scene (can ignore for now, make separate ticket)

[video1] video of a few issues: https://github.com/3DStreet/3dstreet-editor/assets/470477/d654634a-d728-4455-abcb-0742c20b6fe5

[rostyslavvnahornyi]

Hi, @kfarr!

We're currently facing an issue with pagination on the production environment due to the outdated webpack configuration. The old webpack configuration doesn't read environment variables, which is the root cause of the pagination problem.

The pagination is now functioning with the new webpack configuration. You can see it here

Previously, I had separated JS files in the dist directory to optimize the bundle. However, I realized there could be deployment issues, so I commented out the corresponding webpack plugin. As a result, we now have only one JS file in the dist directory.

I fixed the bug with the spinner in the unauthorized "My Scenes" tab.

I also reverted the changes back to "aframe-inspector-opened."

Regarding the "minor fixes" commit, I will provide more accurate commit names in the future. But for understanding this commit was related to resolving merge conflicts.

In conclusion, @kfarr, could you please review the firebase security rules for prod configuration and compare them with the dev environment? I suspect the main issue lies in the webpack configuration. If there are any problems with pagination on the production environment, we might need to adjust the webpack configuration for production. It seems that we don't encounter issues with the development configuration, but there are specific issues with the old production webpack configuration.

If you have any questions, I'll be glad to answer them!

[kfarr]

@rostyslavvnahornyi good news and bad news:

• good news: the build issue is resolved, for example I now confirm that the issue with spinner for logged-out user is resolved which for me confirms the production build and local serving step works
• bad news: pagination simply does not work as you assert it does. Why does it appear to work on dev? The database size on dev is approx 1MB. So upon each button press it loads 1MB. On production, the DB is 7MB. So upon each button press is load 7MB.
• The critical issue remains that the pagination logic is not working as expected -- it is always downloading the entire database with each press of the load more button and upon initial modal open. Are you including bandwidth meter in your testing? How can I better empower you to do this testing prior to providing for review?

This returns us to the 4 options I presented to Viktoria:

1. relax and give Rosty another few days to figure it out, I will be traveling anyway so it will be hard to review something again until Thursday or Friday
2. asking for help (and paying for hours) from another senior engineer at Zade
3. get on a call to discuss challenges or even tradeoffs with potential solutions.
4. don't let sunk cost fallacy force us to continue down this path, open to considering a new clean PR for issue 304. It is ok to admit that the initial attempt needs to be reconsidered.

[rostyslavvnahornyi]

Hi @kfarr,

Could you please check the last commit? A more qualified engineer and I found a problem with pagination and we decided to solve it by changing from client pagination to server pagination. If you encounter any further issues with pagination, the probable cause might be related to the production configuration.

I hope this helps! Let me know if you have any more questions.

[kfarr]

@rostyslavvnahornyi the new logic works as expected however there are many issues that were caused by changes unrelated to pagination that you were attempting in this PR. The changes may be valid but should not be in this PR.

Instead, please create a new PR with ONLY the changes related to this pagination ticket. Other deployment issues should be addressed through other tickets or PRs.

[rostyslavvnahornyi]

Hi @kfarr ,

Pagination is not working with the old configuration. The main issue in the production configuration is that it’s controlled and initiated by the “webpack dev server” package, which leads to errors.

I propose using a best practice approach, which involves separating the webpack configurations for development and production. This way, we can better manage the configurations.

You can see the problem demonstrated in this video.

[kfarr]

@rostyslavvnahornyi I'm sorry to be annoyingly insistent upon this, and I'm not disputing your statement about deployment change being required, but please regardless split this in 2 clean PRs:

• First: infinity scroll using this new "server side pagination" method as its own clean PR (I will test against dev only)
• Second: deployment change as its own clean PR (I will test against dev + production)

[rostyslavvnahornyi]

Hello @kfarr !

So, you can this PRs here feature/scenes-modal-pagination feature/webpack-prod

[kfarr]

Thanks Rosty! Highly appreciated

On Thu, Nov 23, 2023 at 1:11 AM Rostyslav Nahornyi @.***> wrote:

Hello @kfarr https://github.com/kfarr !

So, you can this PRs here feature/scenes-modal-pagination https://github.com/3DStreet/3dstreet-editor/pull/347 feature/webpack-prod https://github.com/3DStreet/3dstreet-editor/pull/346

— Reply to this email directly, view it on GitHub https://github.com/3DStreet/3dstreet-editor/pull/320#issuecomment-1824035480, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADS3TMGQJNRTVGKIGTP7K3YF4HLFAVCNFSM6AAAAAA6PNUTDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRUGAZTKNBYGA . You are receiving this because you were mentioned.Message ID: @.***>

[kfarr]

confirmed this worked as expected split into 2 separate PRs, closing this combined one