-
Notifications
You must be signed in to change notification settings - Fork 25.2k
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
HttpContext/IHttpContextAccessor article #34592
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me for Blazor. We should get signoff from the SignalR folks as well.
@BrennanConroy for a final look, too. I added a paragraph to address your feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wording might want some more massaging, but don't want to block on tiny nits, this is a good improvement.
I'd prefer to work on it further then. These get left for years 😄. Best to make it as good as possible. I'll try again. |
Hummmm 🤔 ... I took another look. It sounds good to me. What specifically don't you like? The line is a bit long, so I could break it into two points of a list.
|
Maybe? |
Fixes #34587
Fixes #14974
NoTeS
Includes trying to close the issue you, Javier, opened for the SignalR doc set.
Not sure on the placement of the SignalR article. Did you prefer just a section? Where do you want the article in the ToC, or where do you want the section if I take that content and make it into a section? IMO, Blazor should have this guidance surfaced better than a section. Pop open the whole Blazor article on the DIFF to review the entire Blazor article. This gets rid of the INCLUDE that I was using before that was burying this content ... devs aren't always finding it as a section.UPDATE: Yes! This is the direction that we should go in.
I'd like to keep the "advanced" cross-links. Dev are asking for more details, and the convos in those issues explain quite a bit, especially the discussion that you, Mackinnion, Steve, and I had on this subject. I hope we can keep it with the lingo that I placed, explaining that it's for "advanced" scenarios and that most devs won't need to look at it. However, I'm ready to 🔪 those cross-links if you want ... I'm just concerned that even more issues will come in if we do.
Internal previews