Changed text based TagHelpers to rendering Chromes with internal span not div#40
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where text-based TagHelpers incorrectly rendered chrome wrappers using div tags instead of span tags, which caused rendering issues when placed inside a p tag. Key changes include:
- Replacing with for opening and closing chrome wrappers in both TextFieldTagHelper and RichTextTagHelper.
- Updating corresponding unit tests to expect tags.
- Reordering using directives in RichTextTagHelperFixture for consistency.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/TextFieldTagHelperFixture.cs | Updated assertion to expect wrappers in rendered output |
| tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/RichTextTagHelperFixture.cs | Updated assertion to expect wrappers and reordered using directives |
| src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/TextFieldTagHelper.cs | Changed chrome wrapper tag from div to span |
| src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/RichTextTagHelper.cs | Changed chrome wrapper tag from div to span |
Comments suppressed due to low confidence (1)
tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/RichTextTagHelperFixture.cs:1
- [nitpick] The reordering of using directives in this test file differs from other files; please confirm that this rearrangement aligns with the project's coding style guidelines.
using AutoFixture;
sc-ivanlieckens
left a comment
There was a problem hiding this comment.
So we're using a text level semantic element to group content? A rich text element can contain a lot of various elements, lots of them being invalid inside of a span afaik? It also changes the default style from block to inline... This feels wrong?
sc-ivanlieckens
left a comment
There was a problem hiding this comment.
Approving as temp solution to unblock. JSS and Pages Team contacted for proper resolution.
This fixes the bug where Text based TagHelpers can't render inside of a
ptag, causing all of MetaData editing to break.This aligns with the JSS approach of using a
spantag instead.I also introduced a new
Assertto the appropriate UnitTests as that tag insertion wasn't being tested currently.Testing
Terms
Closes #39