Skip to content

Make enhance navigation easy and predictable not harder. #50887

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

Closed
1 task done
Bartmax opened this issue Sep 23, 2023 · 6 comments
Closed
1 task done

Make enhance navigation easy and predictable not harder. #50887

Bartmax opened this issue Sep 23, 2023 · 6 comments
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@Bartmax
Copy link

Bartmax commented Sep 23, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Seeing #50551 looks like too much magic for something that should have been be a simple data-interactive=false (or enhance-nav=false.) or even a simpler target="_top", specially for redirects. Also this will be a case by case basis, I don't believe you can have a sane default but to be everything the same way (on was a good default).

You also get double request to a non blazor get endpoint by default (no bueno) i firmly believe it's better to fail.

The developer will know better than any algorithm when to use fetch or not.

Describe the solution you'd like

I believe that a consistent fetch/cors error on a redirect/form post with an easy and simple "enhance-nav=false" fix would have been a better solution. I personally know my links/endpoints and I don't want the framework to play smart.

don't you believe that having to enable every other non-enhanced post form but only if it's post, will necessary makes the dev go through documentation to try to understand why one works but not others, i believe it will be a surprise for many.

Hotwire Turbo uses a simple data-turbo=false approach, works great and even easily supports childs.

I think if you are going to stick with this "magic" default, five us some way to configure so we can make it consistent like,

EnhanceNavigation = EnhaceNavigation.None | EnhaceNavigation.All | etc

Additional context

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Sep 23, 2023
@marinasundstrom
Copy link

marinasundstrom commented Sep 23, 2023

Hi, @Bartmax. You should be able to disable enhanced navigation entirely like this.
I think this will be in RC 2.

<script src="{BLAZOR SCRIPT}" autostart="false"></script>
<script>
  Blazor.start({
    ssr: { disableDomPreservation: true }
  });
</script>

There will also be a way to disable per element. I don't know about attributes though.

I do sympathize with you, as a fellow community member, that it all might be muddled for now. Hopefully, it will be clear soon.

@Bartmax
Copy link
Author

Bartmax commented Sep 23, 2023

That's not what Im in favor of. I want to disable the logic on link/forms not the enhance navigation.

I would like to configure so all links have enabled by default or disable by default and then decide myself if I want to override this setting in a component by component case scenario.

Disabling the whole thing is something else and I'm not interested in.

@marinasundstrom marinasundstrom added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Sep 23, 2023
@Bartmax
Copy link
Author

Bartmax commented Sep 25, 2023

After @SteveSandersonMS replied on the pr itself, I think I did a very poor job on explaining my issue with pr #50551 so I'll try again. Sorry english not my native.

The current state (rc2) of things is:
Forms will always have enhance off, so every form meant to be used within the app will need to add enhanced-nav="on" which already have a lot of requirements in itself (@formname) and is mostly the common use case scenario.
Common use-case: any put/post for 99% of non-indepotent operations like adding a product, add something to a cart, etc.

I believe GET FORMS will do work as fetch by default, but is not clear right now.
Common use-case scenario being search forms with filters and pagination)

Links on the other hand, will be always fetch, no matter if it's a link to www.somedomain.com or myapp.com/weather which is fine, because there's no way the framework will know before hand if somedomain is indeed an app with CORS configured or if it's google.com which will fail miserably which the framework will retry to create a second request based on if the endpoints is blazor or not, which is also wrong for many reasons.
I may want to do fetch request to get HTML content with CORS enable from another app not necessarily need to be a blazor endpoints and still may want to use fetch/enhance (this PR kills this option and I need to do that request manually).

Form Posts that need full page reload will fail (with or without this pr), and we will need to use enhance=off either way.
Common use-case is cookie authentication with 3rd party. (the recommended apporach for securying a same domain SPA without BFF)

The problem with the #50551 pr of doing enhance based on POST/GET or even DOMAIN related is that it actually adds zero value for the developer. I also believe that doing doble get will not get into developer issues is short-sighted, one may want to prevent this double request altogether and get a failed request instead. (I may be missing something, so in that case, please I would love to know what the real use case is of having that).

As stated on the PR, redirection is way trickier with many use-cases and which I agree.

Trying to pretend to know what's best for the developer to use (or not) enhance navigation doesn't enhance the developer experience in my opinion and instead requires reading documentation, understand what's going on behind the scenes and I believe there's a more straightforward approach that is "do nothing."

Proposed solution:
Let developer configure the default behavior: (you can do whatever mix of options you want, I would suggest ON or OFF, but if you want to have an AUTO option it's fine)
And then just simply have a param for turning on or off in a per link/per tag basis.

So, I can do:
Enhance navigation: OFF
but on my few links that I want to use ti, happy

or

Enhance navigation: ON
and on my needed full request form I can add

...

I don't recommend going the route of Enhance navigation:AUTO, if you do, i will suggest letting the ON be the default?

I will be very easy to communicate to the developer. In case you need to turn off enhance nav, you can do enhance=false and that will be it.

Hope this makes it clearer.

@SteveSandersonMS
Copy link
Member

Let developer configure the default behavior: (you can do whatever mix of options you want, I would suggest ON or OFF, but if you want to have an AUTO option it's fine)
And then just simply have a param for turning on or off in a per link/per tag basis.

That's exactly what we do have. You can put data-enhance-nav="false" (or true) at the root of your document, and then override it with data-enhance-nav="true" (or false) on a per <a>-tag basis if you want. Or you can override it at any level in the hierarchy.

I believe GET FORMS will do work as fetch by default, but is not clear right now.

I'd recommend you try it out and see how well the defaults work for you.

@SteveSandersonMS SteveSandersonMS added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Sep 26, 2023
@ghost ghost added the Status: Resolved label Sep 26, 2023
@ghost
Copy link

ghost commented Sep 27, 2023

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@Bartmax
Copy link
Author

Bartmax commented Oct 14, 2023

Ok used it, and I believe it's a big no. The enhance nav forms should be enabled like everything else by default and fail in dev.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 13, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

3 participants