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.

Sign up for GitHub

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

Jump to bottom

Sections support #46727

Merged
merged 34 commits into from
Mar 8, 2023
Merged

Sections support #46727

merged 34 commits into from
Mar 8, 2023

Conversation

surayya-MS
Copy link
Member

@surayya-MS surayya-MS commented Feb 17, 2023 •
edited
Loading

Sections support

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

API Proposal: #46937

Description

  • Made SectionContent and SectionOutlet public
  • Removed SectionContent.IsDefaultContent
  • Removed Name and used object SectionId as a section identifier
  • Added E2E tests for Sections

Fixes #28182

@surayya-MS surayya-MS requested a review from a team as a code owner February 17, 2023 17:16
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Feb 17, 2023
@build-analysis build-analysis bot mentioned this pull request Feb 17, 2023
Open
5 tasks
@surayya-MS surayya-MS mentioned this pull request Feb 22, 2023
Open
MackinnonBuck
MackinnonBuck reviewed Feb 22, 2023
View reviewed changes
Copy link
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.

This is looking great! I just added a few more comments with some minor thoughts.

Before approving, I would still like to see what others think about this suggestion.

src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor Outdated Show resolved Hide resolved
src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor Outdated Show resolved Hide resolved
src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor Show resolved Hide resolved
src/Components/test/E2ETest/Tests/SectionsTest.cs Outdated Show resolved Hide resolved
surayya-MS and others added 2 commits February 24, 2023 11:59
@surayya-MS @MackinnonBuck
Update src/Components/Components/src/Sections/SectionContent.cs
c42520a 
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
@surayya-MS
fix
d3352be
MackinnonBuck
MackinnonBuck approved these changes Feb 28, 2023
View reviewed changes
Copy link
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.

This is looking great 🎉 Thank you for your perseverance on this!

I have a few remaining nits but nothing that should block the approval of this PR. I think this is ready to go after all tests pass (and the API gets approved).

src/Components/Components/src/Sections/SectionContent.cs Outdated Show resolved Hide resolved
src/Components/Components/src/Sections/SectionOutlet.cs Outdated Show resolved Hide resolved
src/Components/Components/src/Sections/SectionOutlet.cs Outdated Show resolved Hide resolved
src/Components/Components/src/Sections/SectionRegistry.cs Outdated Show resolved Hide resolved
src/Components/Web/src/Head/HeadOutlet.cs Outdated Show resolved Hide resolved
src/Components/test/E2ETest/Tests/SectionsTest.cs Outdated Show resolved Hide resolved
src/Components/test/testassets/BasicTestApp/SectionsTest/ParentComponentWithTwoChildren.razor Outdated Show resolved Hide resolved
surayya-MS and others added 4 commits March 1, 2023 09:49
@surayya-MS @MackinnonBuck
Update src/Components/test/E2ETest/Tests/SectionsTest.cs
495bff4 
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
@surayya-MS @MackinnonBuck
Update src/Components/Web/src/Head/HeadOutlet.cs
4742a2d 
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
@surayya-MS @MackinnonBuck
Update src/Components/Components/src/Sections/SectionOutlet.cs
7323ca1 
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
@surayya-MS @MackinnonBuck
Update src/Components/Components/src/Sections/SectionContent.cs
b7be8b4 
Co-authored-by: Mackinnon Buck <mackinnon.buck@gmail.com>
@surayya-MS surayya-MS enabled auto-merge (squash) March 6, 2023 19:26
@surayya-MS surayya-MS disabled auto-merge March 7, 2023 14:40
@surayya-MS surayya-MS enabled auto-merge (squash) March 8, 2023 16:22
@surayya-MS surayya-MS merged commit 83dad2c into dotnet:main Mar 8, 2023
@ghost ghost added this to the 8.0-preview3 milestone Mar 8, 2023
@guardrex guardrex mentioned this pull request Mar 8, 2023
Closed
@surayya-MS surayya-MS deleted the sectionsSupport branch May 15, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nick-Stanton Nick-Stanton Nick-Stanton left review comments

@MackinnonBuck MackinnonBuck MackinnonBuck approved these changes

@SteveSandersonMS SteveSandersonMS SteveSandersonMS left review comments

Assignees
No one assigned
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Milestone
8.0-preview3
Development

Successfully merging this pull request may close these issues.

Sections support in Blazor
4 participants
@surayya-MS @SteveSandersonMS @MackinnonBuck @Nick-Stanton