-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Feature: single block property editor #20098
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
Conversation
src/Umbraco.Infrastructure/PropertyEditors/SingleBlockPropertyEditor.cs
Outdated
Show resolved
Hide resolved
src/Umbraco.Infrastructure/PropertyEditors/SingleBlockPropertyEditor.cs
Outdated
Show resolved
Hide resolved
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.
Excellent work @Migaroez 😍
I've taken the liberty to align the naming of all tests (using underscores as most of the tests were with underscores). I know we don't really do this consequently throughout the codebase, but... call me nitpicking 😆
There are a few TODOs to look into as well.
Lastly - I think you forgot to commit default.cshtml
for single block rendering. Makes sense since the views folder is git-ignored, but it yields a good ol' rendering exception when one attempts to put the SingleBlockTemplateExtensions
to use.
To test the rendering, I created my own default.cshtml
in /Views/Partials/singleblock/
with this content:
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<Umbraco.Cms.Core.Models.Blocks.BlockListItem>
<div class="umb-single-block-list">
@await Html.PartialAsync("singleblock/Components/" + Model.Content.ContentType.Alias, Model)
</div>
If that looks about right from a functional perspective, the rendering tests out fine 👍 it also works strongly typed with models builder types, as one would expect:
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<Umbraco.Cms.Core.Models.Blocks.BlockListItem<MyElement>>
<p>@Model.Content.Title<p>
One last thing I'm wondering about is this (from the PR description):
Create a view for your element type in either views/partials/singleblock/components OR views/partials/blocklist/components.
I can't see how the SingleBlockTemplateExtensions
would support views in the blocklist/components
folder? I tried it out, of course, by moving my view in there (from singleblock/components
). As expected, I got an InvalidOperationException
with The partial view 'singleblock/Components/myElement' was not found
.
I don't think it's a required feature for the extensions to pick up on views from either folder, so unless you were actively planning on supporting that, I believe the PR description should just be amended to omit this?
@kjac I have added the missing file and hopefully it then makes sense. I added this functionality to allow for easier migrations. |
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 👍
Views in blocklist/components
now work, and views in singleblock/components
take precedence in case of matching file names - entirely as expected.
...and indexing works too 😄
Happy to approve!
Just noticed the PR is targeting main
--- dismissing the approval while clarifying the intended target version of this feature.
Also check whether either block of configured blocks can be picked and used from a data perspective
Comment improvements, remove licensing in file
d728baa
to
838b0b6
Compare
Rebased and retargeted to v17 |
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 and tests out good too 👍
Description
This PR introduces a new Single Block Property editor that has a similar data format and behaviour as the Block List Property Editor but will only ever accept 1 block.
A lot of the implemenations listed below are based upon the blocklist/block...base classes. This might in some places not be the most optimal way to handle things, but it does mean we can reuse and there for keep in sync a lot of the more complex logic of these propertyEditors and there data structure/validation. The biggest thing that you might notice is that the underlying data value of the block (even though it is stripped down) still uses an array.
The following things have been changed/added
The following things will be done in future PR's
Example Testing setup
views/partials/singleblock/components
ORviews/partials/blocklist/components
. Example: