ui5-community / bestofui5-website

"Best of UI5" is the new entry page for the ui5-community.
https://bestofui5.org
Apache License 2.0
27 stars 3 forks source link

feat: save scroll state for each view #260

Closed marianfoo closed 2 years ago

marianfoo commented 2 years ago

Changes

Thanks @wridgeu for your suggestion, i hacked a little around. WDYT? Animation

marianfoo commented 2 years ago

Works. However I'm not a fan of these inside-out searches via getParent(). Sometimes they just seem unavoidable though and might even need to be made recursively.

Yeah, your right. We´ll find a solution here, it was just a hack to make it work. I think i need just the first two statements anyway:

I think i can delete the other statements.

What do you think about the generel solution? Is there any edge case i might have missed?

wridgeu commented 2 years ago

What do you think about the generel solution?

I like it. The only "proper" alternative that comes to mind, would be adjusting way too much ~ i.e. the entire view and routing setup I think and even then I am still not 100% sure if that'd fix the issue scrolling behavior. So yeah, I really like it.

Is there any edge case i might have missed?

I did not find any (for now 😁).

Regarding the use of getParent(), as we know how the app is built and there is not too much crazy dynamics going on I think it is fine for now, even still it does make it all somewhat brittle but you know all that already.

I think i need just the first two statements anyway:

That was my observation as well. :)

So all in all I'd say LGTM for now, and I'll approve it. :)