Skip to content

ScrollListener: Add report rate and GetCurrentScrollDataAsync#12183

Merged
danielchalmers merged 21 commits intoMudBlazor:devfrom
91378246:feature/add-advanced-service-example
Jan 18, 2026
Merged

ScrollListener: Add report rate and GetCurrentScrollDataAsync#12183
danielchalmers merged 21 commits intoMudBlazor:devfrom
91378246:feature/add-advanced-service-example

Conversation

@91378246
Copy link
Contributor

@91378246 91378246 commented Dec 1, 2025 •
edited
Loading

This PR adds two new properties to the IScrollListener:

    /// <summary>
    /// The rate at which the <see cref="IScrollListener"/> will report scroll position changes (in milliseconds).
    /// </summary>
    /// <remarks>
    /// Defaults to <c>100</c>.
    /// </remarks>
    public int ReportRateMs { get; set; }

    /// <summary>
    /// Whether to fire the <see cref="OnScroll"/> event immediately after creation.
    /// </summary>
    /// <remarks>
    /// Defaults to <c>false</c>.
    /// </remarks>
    public bool FireOnStart { get; set; }

As well as an example using these:
scroll

@mudbot mudbot bot added the enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library label Dec 1, 2025
@91378246
Copy link
Contributor Author

91378246 commented Dec 1, 2025

Currently a draft as I just cannot find any way to make this slider full height:
image

=> Please advise

@mudbot mudbot bot added the awaiting triage Needs maintainer review or assistance label Dec 1, 2025
@versile2
Copy link
Contributor

versile2 commented Dec 8, 2025

Currently a draft as I just cannot find any way to make this slider full height: image

=> Please advise

Currently a draft as I just cannot find any way to make this slider full height: image

=> Please advise

will take a look.

@versile2
Copy link
Contributor

versile2 commented Dec 9, 2025

So the root of the problem is this component was written before there was wide browser acceptance for vertical positioned sliders. Unfortunately that means unless the slider is allowed to grow to 100% width it will not show the entire slider since it's calculating it's size horizontally then shifting via transform: 270... I can make modifications for it to sort of work, but it does not scale when browsers are resized etc so I'll avoid that for now.

I don't believe this is an easy fix, I've got at least 3 hours on trying to create a work around and have not been completely successful so I believe I'm going to need to alter the component for correct css for modern browsers. (As a reference you can't run Blazor on browser's that do not support modern input slider vertical configurations so no fear there.)

I cannot think of an alternate approach except using a Slider Control without writing a bunch of javascript. (Remember this is supposed to only use Javascript when absolutely necessary). I can probably get modifications to MudSlider done soon but after review time it could be 2-3 weeks. I am going to attempt a workaround I wanted to avoid now and will update you tomorrow.

@versile2
Copy link
Contributor

versile2 commented Dec 9, 2025

That's a working slider. It's not styled like I wanted but I think it's pretty close to what you want. Or will at least get you there.

@91378246
Copy link
Contributor Author

91378246 commented Dec 9, 2025

@versile2 Thank you for looking into that but sadly this introduced a bug where it always jumps back to 50%:
slider

If you want to rework the existing component anyway should I maybe just use the small original slider for now and you ping me once you have committed the changes so that I then adjust the example afterwards?

@versile2
Copy link
Contributor

versile2 commented Dec 9, 2025

This is a workaround using the MudSlider component. Note I did not have the issue you described in the other. This one requires it's container to be 100% width (so it can transform).

@91378246 91378246 marked this pull request as ready for review December 10, 2025 09:08
versile2
versile2 requested changes Dec 11, 2025
View reviewed changes
@91378246 91378246 requested a review from versile2 December 12, 2025 15:18
@versile2 versile2 requested a review from Copilot December 12, 2025 15:48
Copilot AI reviewed Dec 12, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the IScrollListener service by adding configurable throttle rate and initial scroll event firing capabilities. These additions provide developers with more control over scroll event handling behavior.

Key Changes:

  • Added ReportRateMs property to configure the scroll event throttle rate (default: 100ms)
  • Added FireOnStart property to optionally fire scroll events immediately upon listener creation (default: false)
  • Updated the JavaScript implementation to support these new parameters

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
mudScrollListener.js Updated to accept and use configurable report rate and fire-on-start behavior
IScrollListener.cs Added new properties ReportRateMs and FireOnStart to the interface
ScrollListener.cs Implemented new properties with initial event caching and subscriber notification logic
IScrollListenerFactory.cs Updated factory method signature to include new parameters
ScrollListenerFactory.cs Updated factory implementation to pass new parameters to ScrollListener constructor
MockScrollServices.cs Updated mocks to support new properties and parameters
ServicesPage.razor Reorganized documentation to separate basic and advanced examples
ScrollListenerExampleAdvanced.razor Added new advanced example demonstrating the new features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

versile2
versile2 requested changes Dec 12, 2025
View reviewed changes
Copy link
Contributor

@versile2 versile2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, copilot 2 suggestions were relevant and of course a preference on the Create factory method. I know Kuro has this in the block to be rewritten at some point but no telling what schedule would leave enough time for that open! Also, don't hesitate to tag me to review any that you have that are old, we have a list of things to do for V9 and its easy to get laser focused and miss good PRs.

@91378246
Copy link
Contributor Author

91378246 commented Dec 12, 2025 •
edited
Loading

@versile2 #12260 (comment) => should I redo the namespace change or no?

@versile2
Copy link
Contributor

versile2 commented Dec 12, 2025 •
edited
Loading

@versile2 #12260 (comment) => should I redo the namespace change or no?

not until I get a definitive answer on the standard. And once we do we'll change the entire project at once likely so we don't have the "cant figure out the changes" issue.

Edit: You followed up got good answer lol. New components it's fine try not to change the old.

This was referenced Feb 23, 2026
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Closed
Merged
Closed
Open
Closed
Closed
Closed
Merged
Closed
Closed
Merged
Closed
Open
Open
Closed
Open
Closed
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@ScarletKuro ScarletKuro ScarletKuro approved these changes

@versile2 versile2 versile2 approved these changes

Assignees

No one assigned

Labels

breaking change This change will require consumer code updates enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@91378246 @versile2 @ScarletKuro @danielchalmers