Skip to content

Docs: Add Services page#11967

Merged
danielchalmers merged 8 commits intoMudBlazor:devfrom
91378246:feature/add-services-into-docs
Oct 19, 2025
Merged

Docs: Add Services page#11967
danielchalmers merged 8 commits intoMudBlazor:devfrom
91378246:feature/add-services-into-docs

Conversation

@91378246
Copy link
Contributor

MudBlazor provides several general useful services, but they aren't mentioned within the documentation. This PR adds a new point Services under Features. The Services page describes currently following services:

  • BrowserViewportService
  • ResizeObserver
  • ScrollManager
  • ScrollListener

These where the services that I identified as being non-specific enough to be general useful outside of the MudBlazor project.

image image image image

@mudbot mudbot bot added the docs Updates/improvements to project documentation that do not affect core library logic label Oct 17, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces documentation pages for several MudBlazor services, including BrowserViewportService, ResizeObserver, ScrollManager, and ScrollListener. The changes involve adding new Razor components to demonstrate the usage of these services and updating the MenuService to include a link to the new 'Services' documentation page. I have identified a potential issue with the ResizeObserverExample.razor component where the OnResized event is not properly unsubscribed, which could lead to memory leaks. I have also identified an issue with the ScrollListener where the selector is set in the OnAfterRenderAsync method, which might cause issues if the component is re-rendered.

Copy link
Contributor

@jperson2000 jperson2000 left a comment

Choose a reason for hiding this comment

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

I think this is a great addition to the docs! I had just a few comments to consider.

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 18, 2025

  • BrowserViewportService

It does under MudHidden component:
https://github.com/MudBlazor/MudBlazor/blob/47e4ff38716aa367de9bfc9833a87d4066b39487/src/MudBlazor.Docs/Pages/Components/Hidden/Examples/BrowserResizeEventExample.razor

If we want to have a dedicated page for this, probably should show two ways of using the service, with implementing IBrowserViewportObserver like example above and with using lambda like your example.

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 18, 2025

Another issue is that we don't actually recommend using:

IResizeObserver
IScrollListener

Instead, we recommend using:

IResizeObserverFactory
IScrollListenerFactory

Due to this problem #4631
In short IResizeObserver, IScrollListener implement IDisposable and when you register such services as transient (which they are), the MS DI container captures these services and never actually releases them, leading to a memory leak: https://learn.microsoft.com/en-us/answers/questions/2336787/why-does-di-container-retain-references-to-transie
MS relevant code:
https://github.com/dotnet/runtime/blob/f0db8f9d9668ba03897739152d02623acbd04466/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceProviderEngineScope.cs#L78
https://github.com/dotnet/runtime/blob/f0db8f9d9668ba03897739152d02623acbd04466/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ServiceCallSite.cs#L26

The IScrollManager does not implement IDisposable, and the services IBrowserViewportService, IKeyInterceptorService, and IPopoverService are newly implemented with this issue in mind. They are not registered as transient services.

@mudbot mudbot bot added the needs: changes A maintainer has asked for further modifications to be made to this pull request label Oct 18, 2025
@91378246
Copy link
Contributor Author

@ScarletKuro Thank you, didn't know that, thought the framework would handle the disposing of all disposable services => updated the examples to use the factories.

@mudbot mudbot bot removed the needs: changes A maintainer has asked for further modifications to be made to this pull request label Oct 18, 2025
@danielchalmers danielchalmers changed the title Add service section to docs Docs: Add Services page Oct 19, 2025
@danielchalmers danielchalmers merged commit 6aa12a2 into MudBlazor:dev Oct 19, 2025
6 checks passed
@versile2
Copy link
Contributor

I know I'm late to the party, and this is fantastic and sorely needed to be added to the main documentation. However, I would like to see ScrollManager fixed to not allow "Lock Scroll" to happen more than once. Currently ScrollManager can "Lock Scroll" an unlimited number of times and you will need to "unlock" scroll that many times to unlock it. It should track it's locked and ignore additional lock requests in my opinion. Similar to MudOverlay's lockscroll feature.

Also the ScrollListener example is a bit wonky, can't scroll the test div there's no scrollbar:
image

Additionally it would be nice to show scroll listener for "page" or body scroll? (passing null to selector sets this to "document", passing a selector instead of null would replace the .Selector line. Basic usage is still good enough but we are leaving a bit of meat on the bone by not expanding it.

@91378246
Copy link
Contributor Author

91378246 commented Oct 20, 2025

@versile2 I'm not quite sure why this doesn't display a scrollbar for you (Server, Chromium):
scrollbar
I just answered it myself by typing "Chromium"...
No idea why this doesn't work in Firefox => will look into it
Ah it does work, the background-choice just wasn't the best for light mode in Firefox:
image
image

  • ScrollManager: Do you mean I should track this within the example and adapt the example ui or do you refer to updating the service itself?
  • ScrollListener: Ah, you are right, sry, oversight by me

=> As the current one is closed: Should I just create a new small branch "Service page additions"?

@ScarletKuro
Copy link
Member

@versile2 / @91378246 feel free to add adjustments PR.

@versile2
Copy link
Contributor

Yes new PR for adjustments.
I'm testing on Chrome but yes I think the background is the issue. Also Would be "prettier" if the text didn't shift when it adds a decimal place. But not required.
The ScrollManager is fine for this PR and probably out of scope for your change. When time permits if you have time it would be a nice fix. Not required for what you've done.

@91378246
Copy link
Contributor Author

@versile2 Added a PR: #11977. Please tell me if I understood you correctly. Regarding the scroll position of the body: I couldn't get it work (attaching to html/body/mud-layout etc. does nothing and if I try document it throws).

@91378246 91378246 deleted the feature/add-services-into-docs branch November 4, 2025 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Updates/improvements to project documentation that do not affect core library logic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants