Skip to content

Add SectionName to SectionOutlet and SectionContent#47221

Merged
surayya-MS merged 4 commits intodotnet:mainfrom
surayya-MS:sectionName
Mar 17, 2023
Merged

Add SectionName to SectionOutlet and SectionContent#47221
surayya-MS merged 4 commits intodotnet:mainfrom
surayya-MS:sectionName

Conversation

@surayya-MS
Copy link
Copy Markdown
Member

@surayya-MS surayya-MS commented Mar 15, 2023

Add SectionName to SectionOutlet and SectionContent

Description

New API Proposal for Sections Support. Adding SectionName

Fixes #46937

@surayya-MS surayya-MS requested a review from a team as a code owner March 15, 2023 15:49
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Mar 15, 2023
Comment thread src/Components/Components/src/Sections/SectionContent.cs Outdated
Copy link
Copy Markdown
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Looking great so far!

Comment thread src/Components/Components/src/Sections/SectionOutlet.cs Outdated
Comment thread src/Components/Components/src/Sections/SectionContent.cs Outdated
Comment thread src/Components/Components/src/Sections/SectionContent.cs Outdated
Comment thread src/Components/Components/src/Sections/SectionOutlet.cs Outdated
Comment thread src/Components/Components/src/Sections/SectionOutlet.cs Outdated
@surayya-MS surayya-MS enabled auto-merge (squash) March 17, 2023 14:13
Copy link
Copy Markdown
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Looks great! 🎉

@surayya-MS surayya-MS merged commit f7e68a8 into dotnet:main Mar 17, 2023
@ghost ghost added this to the 8.0-preview3 milestone Mar 17, 2023
}

_registry.AddProvider(_identifier, this, IsDefaultContent);
_registeredIdentifier = SectionId;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Now that you also can use SectionName to identify a section _registeredIdentifier should be set to identifier instead of SectionId. Otherwise when using the SectionName it would be null, and RemoveProvider isn't called when it should.

The same issue exists in SectionOutlet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Flachdachs this is a closed PR, so could you log an issue with details around your concern?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Of course. Actually my intention was to open an issue. I visited the page with the changes anonymously. It offered a "New Issue" button. But as I opened the same URL as logged in user the button was replaced with a Create Review button.

@surayya-MS surayya-MS deleted the sectionName branch May 15, 2023 14:37
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Blazor Sections API Proposal

4 participants