Skip to content
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

Enhance Navigation works horribly, I would suggest drop it all and re do it again. #54982

Closed
1 task done
Bartmax opened this issue Apr 5, 2024 · 7 comments
Closed
1 task done
Labels
area-blazor Includes: Blazor, Razor Components investigate product-feedback

Comments

@Bartmax
Copy link
Contributor

Bartmax commented Apr 5, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I already stated few things about enhance nav and why many decisions are very bad developer and user wise. The response was always "nah, it's fine." Well, still sucks.

I can't state everything it's wrong, it's too many list of things, i would redo everything, specially when there is no need or intention to actually look at the real problem and just say "its ok, bla bla bla"

anchor tags with enhance-nav off works different than navlinks with enhance-nav off
localhost works different with enahnce-nav off than production/domain enhance-nav off

enhance nav doesn't work at all as expected when usin SSR

you can still say : "it's fine the design is good", unfortunately that doesn't make it real.

amazingly, hotwire turbo is a way simpler design, not only works as expected, it does works every single time, and there's not even 1 line of code of magic. It just believe in the developer.

So, going to production, having a pain with enhance nav, i turned off and im using turbo.

For anyone dealing with all wilderness of enhance nav, i can only recommend to just drop it, it's buggy software as buggy as it can be.

If there's a change on NET team mind and want to look at the issues and legitimately interested on fixing, there are a few (2-3) simple design decision changes that will solve ALL problems at once.

If you are still not convinced but still want to make it work, I can compromise on making 1 repo for every major problem it has with as much detail as i can.

Expected Behavior

That works

Steps To Reproduce

turn it on

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Apr 5, 2024
@MackinnonBuck
Copy link
Member

anchor tags with enhance-nav off works different than navlinks with enhance-nav off

How does it work differently?

localhost works different with enahnce-nav off than production/domain enhance-nav off

How is it different? What behavior are you seeing?

I can compromise on making 1 repo for every major problem it has with as much detail as i can.

That would be great. Ideally it would include comments that make it clear what the issues are.

there are a few (2-3) simple design decision changes that will solve ALL problems at once.

Could you be more precise about this?

@MackinnonBuck MackinnonBuck added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Apr 8, 2024
@Bartmax
Copy link
Contributor Author

Bartmax commented Apr 11, 2024

anchor tags with enhance-nav off works different than navlinks with enhance-nav off

How does it work differently?

On localhost, having this two supposedly equivalent links:
<a data-enhance-nav="false" href="/#somesection">link</a>
vs
<NavLink data-enhance-nav="false" Href="/#somesection">link</NavLink>

the <a> tag does a fetch request no matter what.
the <NavLink> tag doesn't do a fetch on localhost but does a fetch on azure (I believe is DEBUG vs RELEASE, but I haven't tested it)

there's also this issue: #52205

localhost works different with enahnce-nav off than production/domain enhance-nav off

How is it different? What behavior are you seeing?

Turning Enhance Nav to false shows different results on localhost debug than azure release.

I can compromise on making 1 repo for every major problem it has with as much detail as i can.

That would be great. Ideally it would include comments that make it clear what the issues are.
This will take some time; I'll try to accommodate to make those. I let you know.

there are a few (2-3) simple design decision changes that will solve ALL problems at once.

Could you be more precise about this?

Sure, I tried when the feature was on development at #50887 but it's hard for me to be clear, I'm normally misunderstood so I'm sorry for that, english is not my native.
I'll try to explain again what I believe would have been way better design decisions and a few examples of why the current state doesn't work for me.

The simple design decisions are removing the "magic" / "try to be clever" about when to do one thing or the other. There was also a big "decision" issue from @SteveSandersonMS (#50551 (comment) and #50551 (comment)) explaining the challenges of when to do what with Enhanced Nav, the TLDR was that FORM GET was totally forgotten in that scenario, FORM POST worked with a try to do fetch except is to another domain which does not and never double request while anchor will double request try a fetch if that not work goes with normal request.

Even the TLDR being that complex should raise some flags, but the team ignored.

We developers aren't stupid and normally we have different reasons to do A or B stuff, so instead of that I suggested to don't do anything except explicit by the developer. So, if I set enhance = true, then do fetch if not do normal request and dont ever "try to see if it works". All "errors" will show on dev time and would be easy to fix.
Saying that, that "magic" broken the authentication logic when you need to post to another domain and do the auth dancing and redirect back because of enhance nav (at that time, turning off wasn't possible).

Now you can turn off enhance nav, good, except that it doesn't work, and it just set all childs, there's no donut enhance nav, which also brokes some scenarios I described on another issue about modal, where you want to do (for example):

<form enhance="false">
...
<a href="/tab1" enhance=true>Tab 1</a>
<a href="/tab2" enhance=true>Tab 2</a>
</form>

👆 this doesn't work. Why someone would want to do that? for example, when you want to do ONLY SSR, that's a desired scenario.
The oposite is also true.

Other design decision is limited replacement. (#54117)
Let's say I can use SSR and add an item to my shopping cart but doing that will fetch the whole page and I may only want to update the "cart" section. That would be a good decision to limit the response to a specific ID. This is not possible, the solution for this was to have a data-permanent on what do you want to not refresh. That doesn't work becase again of hierarchy where if I want to keep the <body data-permanent=true> intact, then there's no way I can use <div id=cart data-permanente=false>
This is a pain when working with javascript libraries that changes the DOM outside of blazor
I also think this is a bad design decision.

Last but not least, at first when you navigate to another page, the scroll went straight to the top and many people complained that switching tabs wasn't expecting to trigger a scroll event.
The solution was to make no nav event trigger a scroll, which now breaks all "normal" SSR navigation pages.

Ultimately, fragments as well as FORM GET, basic HTML behavior, were not taken into consideration as far as I know or greatly oversighted which would be a bonus point for those to work as expected, which aren't right now.

All that issues together forces me to turn off enhance nav entirely and I cannot use it.
It's not easy to predict when and where will it work and how.
It's not possible to do many simple but desired stuff (like donut fetch)
etc.

My humble opinion is that Enhance Navigation should work flawlessly on a SSR only site without any interactivity (nor webassembly nor server) first, THEN move to support the other interactivity.
There's too much power on SSR alone, that having to turn it off it's sad.

Allow the developer to opt in and opt out of features, and always works predictable is so much worth it in my opinion, specially it will have less bugs, better to understand and work with and way easy to maintain.

Hope it's clear, of course if you want more detail, I believe I can make all these points (and more, this are a few subset of the biggest problems, but there are more annoyances) with examples, but it will take time. Please, confirm after all this info if you are interested.

Oh, I almost forgot, prg pattern is also a pain with enhance nav on.

If you need any more details or questions, please feel free to ask

@dotnet-policy-service dotnet-policy-service bot added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Apr 11, 2024
@mkArtakMSFT mkArtakMSFT added investigate product-feedback and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Apr 11, 2024
@mkArtakMSFT mkArtakMSFT added this to the .NET 10 Planning milestone Apr 11, 2024
@SteveSandersonMS
Copy link
Member

the tag does a fetch request no matter what.
the tag doesn't do a fetch on localhost but does a fetch on azure (I believe is DEBUG vs RELEASE, but I haven't tested it)

This seems very strange since at runtime, <NavLink> is just an <a> tag so it's hard to imagine how their behavior could differ. The only idea that comes to mind is the casing of Href in your code example (should be href, since it's an HTML attribute and not a component parameter).

If you can post a runnable repro we can have a look.

@SteveSandersonMS
Copy link
Member

Also as a further design point I should add that enhanced nav is not intended as the ultimate design that covers every possible requirement and programming style. It is meant to be straightforward and reliable, so if you find cases where it isn't, we definitely appreciate repros we can look at. I fully accept there are rough spots and inconsistencies today, which we'd love to iron out.

As for cases where you want a whole different approach, that is definitely meant to be possible. For example you should be able to use Blazor SSR with:

  • Turbo
  • HTMX

... and any other JS library in this category, based on your design and functionality preferences.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Apr 15, 2024

Now you can turn off enhance nav, good, except that it doesn't work, and it just set all childs, there's no donut enhance nav, which also brokes some scenarios I described on another issue about modal, where you want to do (for example):
<form enhance="false">
<a href="/tab1" enhance=true>Tab 1</a>
<a href="/tab2" enhance=true>Tab 2</a>
</form>
👆 this doesn't work.

There is "donut enhanced nav" - the syntax is as follows, which I believe does work:

<form method="post" @formname="myform" data-enhance="false">
    <AntiforgeryToken />
    <button type="submit">Submit</button>
    <a href="weather" data-enhance-nav>Weather</a>
</form>

@Bartmax
Copy link
Contributor Author

Bartmax commented Jun 21, 2024

As NET9 preview5 was released, I wanted to give it another try. (spoiler alert: it is still broken)
1 annoying issue was fixed. 👍

I made a repo of the 2 others: Scroll position on page navigation and what I call " donut enhance " that is update a inner part of a component only.

https://github.com/Bartmax/Net9App

One thing not mentioned early, having data-enhance and data-enhance-nav as different attributes one for forms and other for links is confusing. I believe data-enhance will have been better for both cases.

@mkArtakMSFT
Copy link
Member

Thanks for your feedback, @Bartmax.
It looks like we already have an issue tracking the scroll position problem: #51646

Could you please file a separate issue for the problem you refer to as donut enhance and provide more details about the scenario.

@mkArtakMSFT mkArtakMSFT closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components investigate product-feedback
Projects
None yet
Development

No branches or pull requests

4 participants