vladmandic / automatic

SD.Next: Advanced Implementation of Stable Diffusion and other Diffusion-based generative image models
https://github.com/vladmandic/automatic
GNU Affero General Public License v3.0
5.53k stars 405 forks source link

[Issue]: System / Settings: page jumps to top on change in "show all pages" view #2367

Open Tillerz opened 11 months ago

Tillerz commented 11 months ago

Issue Description

When you are in the SD config and have clicked "Show all pages" and then edit something (like toggling a checkbox) the page always scrolls/jumps to the top. It is not happening when you have only one section of the settings open.

Version Platform Description

53d0cb35 Win 11 latest Chrome

Relevant log output

N/A

Backend

Original

Model

SD 1.5

Acknowledgements

brknsoul commented 11 months ago

It seems the settings pages are also jumping when typing, and when applying settings. Forced camera angles sucks. ;-)

vladmandic commented 11 months ago

they do. and i'm really not sure where is that from as its definitely not intentional. if someone wants to spend a bit of time tracing js code (it has to be in js), great. otherwise, this will stay in backlog for a bit.

nCoderGit commented 10 months ago

If I understand it correctly, there are two issues with the settings right now:

Issue 1) When you open SD.Next in the browser, initSettings() runs. However, the MutationsObserver callback doesn't run before you clicked on one of the categories (like "Compute Settings", "Show all pages" etc).

Basically the scrollIntoView() call does what it's supposed to, but it's somehow followed by wiping that settings panel (showAllSettings() or scheduler.js's flush()?), everytime a click or textbox keypress happens and thus scrolls back to a "safe" position.

Issue 2) When you have a textbox in your quicksettings (e.g. directories_filename_pattern) and you type something, there's some really serious lag / rubber banding. This seems to happen because every single keypress triggers onOptionsChanged() which in turn triggers the forEach that calls markIfModified(), which in turn calls scrollIntoView() for every dirty setting.


Wouldn't it be best to just ditch the scrollIntoView() call, anyway?

vladmandic commented 10 months ago

something is causing settings page to scroll to top on any change. since i didn't have time to trace it down, i just added scrollIntoView as a workaround. i'll remove it now due to bad sideeffects, but if you can trace down what is causing scroll in the first place - well, contributions are more than welcome!

nCoderGit commented 10 months ago

Yup, I'll give it a try

nCoderGit commented 10 months ago

I noticed that all TabItem content gets set to style.display=none, which is what causes the scroll.

A simple tab switch from settings_sd to settings_cuda for example, raises only 2 mutation events. When Show all settings is open, there are 17 mutations (all settings tabs except "Show all settings"), and these 17 are raised every time I change some setting or even switch to a completely different tab group like "System info".


Could it be that Gradio just hates it when you try to have more than one TabItem open at the same time, and then trigger a change on any component?

Any idea? (Would trigger_mode=multiple help?)

vladmandic commented 10 months ago

I don't think it's a Gradio specific thing - question is where is display:none being triggered?

nCoderGit commented 10 months ago

1.) Click handler in [Tabs] grafik


2.) → in [Component.js]: make_dirty() → in [scheduler.js]: → flush() → update() ...


3.) → in [TabItem] : grafik

vladmandic commented 10 months ago

argh. so it is in gradio's tab handler :( any suggestions?

nCoderGit commented 10 months ago

Well, I guess we could make "Show all pages" (and settings search) just add/remove some display:block !important class on each setting's TabItem content - that should get rid of the scroll.

Still only a workaround, though, since make_dirty() would still fire on every tab that's not "selected".

nCoderGit commented 10 months ago

I think the cleanest solution would be to make it an accordion layout. And I suppose we can always style that thing into a 2-column grid layout via css if needed, so it resembles our current tabs layout 🤔

vladmandic commented 10 months ago

fair. but that would be a non-trivial task for a relatively small return.