-
Notifications
You must be signed in to change notification settings - Fork 927
Topmost visible section instead of closest above center #2340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Referencing radzenhq#2328, take the top most visible section as url reference instead of the clostest element.top to and above center line of app container
|
Hi @Anspitzen, Thank you for the pull request. For some reason I still see similar behavior as before - reloading the page scrolls to the wrong section. Here is what I see |
|
I did further investigation and it seems that most of the problem is the initial scroll - when the browser itself scrolls to the current anchor. Then our code runs and may detect a different section. Thus I propose this change which simply ignores the first scroll provided the conditions are met: --- a/Radzen.Blazor/wwwroot/Radzen.Blazor.js
+++ b/Radzen.Blazor/wwwroot/Radzen.Blazor.js
@@ -2573,7 +2573,19 @@ window.Radzen = {
const elements = selectors.map(document.querySelector, document);
this.unregisterScrollListener(element);
+ let suppressInitialScroll = false;
+
+ if (selectors.indexOf(location.hash) !== -1) {
+ currentSelector = location.hash;
+ suppressInitialScroll = true;
+ }
+
element.scrollHandler = () => {
+ if (suppressInitialScroll) {
+ suppressInitialScroll = false;
+ ref.invokeMethodAsync('ScrollIntoView', currentSelector);
+ return;
+ }
const center = (container.tagName === 'HTML' ? 0 : container.getBoundingClientRect().top) + container.clientHeight / 2;You can test the changes in the suppress-initial-scroll branch. |
|
Hi @Anspitzen, Could you please test my change in the linked branch? It seems to work fine but I want to be sure your case works too. |
|
I did check it, and it still has some unwanted behaviour. Still thinking on how to possible solve that here. |
|
Another trigger would be when you have some scrolling in a component (like datagrid) I do confess, this handling is pretty unlikely to happen in the wild, but I did it to check when the scrolling scrippt will be triggered. |
|
Those observations are valid but I don't have a solution for them. Sometimes just more than one section is "visible" so there is no clear way to determine what to highlight. Not to mention the case when all items are visible (sections are too short). Perhaps we should just add an option to disable active element tracking on scroll. |
|
I am closing this PR since the proposed solution doesn't seem to address the original problem completely. |

Referencing #2328, take the top most visible section as url reference instead of the clostest element.top to and above center line of app container