search

DavidVollmers / Ignis

The Blazor framework for building modern web applications.
https://ignis.dvolper.dev
MIT License
150 stars 8 forks source link

Implement scroll into view for popup components #54

Closed panoukos41 closed 5 months ago

panoukos41 commented 5 months ago

As discussed in #52 this implements ScrollIntoView in a generic way in AriaPopupExtensions

Implementation

Since each item has an Id it worked for ListBox and Menu by just passing the JSRuntime to the appropriate modified method (I made it optional but maybe a flag would be appropriate).

For Popover I think we would need more changes to provide access to Ids etc. We could also make the extension public for easier access to developers to customise it as they see fit.

The discarding of the task is intentional since the method is already void and at the same time it should not interupt the ui loop if something goes wrong like not finding the element if it was removed from the dom.

To Discuss

While using it I noticed the following: As you go up or down sometimes (probably when scrolling stops) the mouseenter event fires as the mouse enters a new li item producing the following effect (as you go up/down the element below the mouse is selected) msedge_15

Reading around some answers in SO I came accross a solution that suggests to use the mousemove event as the mouseenter works as expected (the mouse entered a new element) but we don't really want it to fire during keyboard scrolling. Here is the result with the mousemove event which does fix the problem: msedge_17

Should I commit the change for the mouseenter too?

DavidVollmers commented 5 months ago

I like the concept! I think it is an easy solution for the problem. I added some comments regarding implementation details and conventions.

As for the mousemove event the problem here is that this would fire a lot of events which will impact performance when the component is server side rendered. Instead we might want to disable the mouseenter logic while scrolling, although that might be difficult to implement.

panoukos41 commented 5 months ago

I can't see any comments, was it somewhere else?

DavidVollmers commented 5 months ago

I had to complete the review, my bad.

panoukos41 commented 5 months ago

If anything else needs to be resolved let me know and thanks for bearing with me 😅

DavidVollmers commented 5 months ago

Looks good to me! I will check it out locally today or latest tomorrow and test a few things between web assembly/server mode but if there are no more concerns I will merge it.

Thank you very much for your contribution so far!