dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.62k stars 25.3k forks source link

[Blazor] include/prerendering.md - Replacing the bad practices with simple demos #32124

Closed hakenr closed 2 months ago

hakenr commented 7 months ago

Description

Is this intentional that the included section includes/prerender.md is full of bad practices and (repeated) explanations why these are bad? Do we want to use this as documentation of what not to do?

If not. I propose to replace the JS interop samples with some simpler ones which do not implicate the need of the "bad practice warnings".

For example, for PrerenderInterop1.razor (which demonstrates the need to wait for divElement field to be populated) we could use something like element.scrollIntoView (https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView) instead of manipulating the DOM.

For PrerenderingInterop2.razor (which demonstrates gathering return value from JS and need for StateHasChanged() to re-render the component with usage of the value gathered) we could use window.getScreenDetails (https://developer.mozilla.org/en-US/docs/Web/API/Window/getScreenDetails) to display screen size in the component.

Page URL

https://learn.microsoft.com/en-us/aspnet/core/blazor/components/lifecycle?view=aspnetcore-8.0#component-initialization-oninitializedasync

Content source URL

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/blazor/components/lifecycle.md

Document ID

4871f4f7-9cf6-ac7a-0f21-c65d34bd1119

Article author

@guardrex

github-actions[bot] commented 7 months ago

😎 March! The gateway to spring! 🌞

A green dinosaur πŸ¦– will be along shortly to assist. Stand-by ........

guardrex commented 7 months ago

Yes, improvements are welcome for these examples.

Generally, WRT avoiding module use and keeping the pollutes the client with global functions warning, we're going to stick with the approach for now. Keep one of these warnings with a cross-link for anything JS interop FN examples that don't use a module.

BTW ... I think this example might be the first JS interop example seen, which is why the exported FN is shown. It's meant to explain just a little bit more than later examples explain about module use. However, please drop that aspect if it seems like it doesn't make sense for your revised examples. I think keeping a cross-link to the JS interop location guidance, which is a new standalone article now, is sufficient.

guardrex commented 2 months ago

@hakenr ... I made it back here πŸ˜….

I like your idea of using scrollIntoView, and I'd like to remove the first example. Two examples of this concept aren't necessary.

How about this .......

window.scrollElementIntoView = (element) => {
  element.scrollIntoView();
  return element.getBoundingClientRect().top;
}

PrerenderedInterop.razor:

@page "/prerendered-interop"
@using Microsoft.AspNetCore.Components
@using Microsoft.JSInterop
@inject IJSRuntime JS

<PageTitle>Prerendered Interop</PageTitle>

<h1>Prerendered Interop Example</h1>

<div @ref="divElement" style="margin-top:2000px">
    Set value via JS interop call: <strong>@scrollPosition</strong>
</div>

@code {
    private ElementReference divElement;
    private double? scrollPosition;

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        if (firstRender && scrollPosition is null)
        {
            scrollPosition = await JS.InvokeAsync<double>(
                "scrollElementIntoView", divElement);

            StateHasChanged();
        }
    }
}

~I have that just about ready to go on a branch with article updates to the INCLUDE file. I'll hold here for your feedback because I'm going to update all of the sample apps with it.~

UPDATE: It's all ready to go on the two PRs. Unless you think there's a problem here, I'm ready to merge them.

hakenr commented 2 months ago

I like the new demos! ;-)