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.

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

WIP|POC:Out of the inline relation rendering for backend #73

Closed
wants to merge 9 commits into from

Conversation

DanielSiepmann
Copy link
Contributor

This is a PoC, not final implementation.

Relates: #68

@DanielSiepmann
Copy link
Contributor Author

I opened this PR so we have an easy way to discuss stuff based on the PoC :)

Classes/Backend/Preview/PreviewRenderer.php Outdated Show resolved Hide resolved
Classes/Backend/Preview/PreviewRenderer.php Outdated Show resolved Hide resolved
Comment on lines 58 to 74
$processedField = $this->processField(
$tcaFieldDefinition->getFieldType(),
$processedField,
$tcaFieldDefinition,
$table,
$depth,
$context
);
if (isset($processedField['__resolvedGrid'])) {
$processedContentBlockData[$tcaFieldDefinition->getIdentifier() . '_grid'] = $processedField['__resolvedGrid'];
$processedField = $processedField['__resolvedField'];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole process feels a bit clunky to me. But I couldn't figure out a better way yet …
Maybe you have ideas, or we can put our heads together.

Copy link
Owner

Choose a reason for hiding this comment

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

I moved the Result into a dedicated struct and added _grids to ContentBlockData. This way it is better separated from "normal" processed fields.

$context,
$column,
$record->_raw,
// $tableName,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be done once we have a new TYPO3 release with https://review.typo3.org/c/Packages/TYPO3.CMS/+/82411

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, perfect!

Comment on lines 73 to 80
$data = $contentBlockDataResolver->buildContentBlockDataObjectRecursive(
$contentElementDefinition,
$contentElementTableDefinition,
$record,
$contentElementTable
$contentElementTable,
0,
$item->getContext(),
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are way to many arguments for my taste.
Maybe we can come up with some other API, encapsulation or something else.

Copy link
Owner

Choose a reason for hiding this comment

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

Mhh, yeah slowly. Could be moved into a single carrier class. Let's first get this done and refactor this at the very end.

@nhovratov
Copy link
Owner

So, I played around a bit and it already works really well. What disturbed me was the dragging possibility was still there for the children. This can't work of course. So I decided to copy the relevant partials and adjust them a bit for our purpose. I think this is ok. When there are changes in the future in this area, the Core is forced to adjust these partials as well, as this is part of the Core then.

You can use it like this now:

<f:render partial="PageLayout/Grid" arguments="{data: data, identifier: 'children', allowEditContent: 1}"/>

ContentBlockData must be provided, as it holds all the grids. Then the identifier of the Relation field as a string. Lastly allowEditContent. This should possibly be just enabled by default.

Comment on lines +77 to +79
if ($name === '_grids') {
return $this->_grids;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that. The only drawback: We now have this always empty in frontend.
Maybe we can adjust that and use this to provide already rendered grids for frontend? I guess EXT:gridelements and EXT:container did the same?
That way it would be the "same" experience in frontend and backend.
Processed holds the data while _grids holds the ready to use stuff for rendering based on context.

We could alter the generation. Do what we do right now in case Context is provided. Use TypoScript rendering otherwise.
Or provide a callback from the outer scope to generate the grid. That way the resolver doesn't need to know inspect and act, but the caller can influence the result.

What do you think @nhovratov?

Copy link
Owner

Choose a reason for hiding this comment

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

This sounds really interesting. This would allow users to alternate between standard rendering (TypoScript) and custom rendering via processed. Let's keep this idea in mind. We should think about a better naming then, I believe. Like "children" or "nestedContent". Idk. Of course this would only make sense for tt_content records, as other record types rarely have a TypoScript rendering definition, altough it would be possible.

@DanielSiepmann
Copy link
Contributor Author

The current state has a small issue with the language flag icon within the grid. It misses some spacing:
image

But the only difference in templates is the drag and drop class.
But Firefox shows there is a missing whitespace which would create the space.

@nhovratov
Copy link
Owner

The current state has a small issue with the language flag icon within the grid. It misses some spacing:
But the only difference in templates is the drag and drop class. But Firefox shows there is a missing whitespace which would create the space.

Yeah, it's the whitespace. I've seen the same issue with EXT:container already. It should be fixed in the Core styling imo, so the space does not rely on space characters.

@nhovratov
Copy link
Owner

FYI Patch for the gap: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82427

@nhovratov
Copy link
Owner

Current state with patches applied to Core

  1. Nested Content Element (works without patches, as it is tt_content)
  2. Generic Record Type (without a type field + StandardContentPreviewRenderer)
  3. Generic Record Type (with type field + StandardContentPreviewRenderer)

Screenshot 2024-01-23 at 22-14-10 Page · Home · Content Blocks TYPO3 CMS 12 4 10

@DanielSiepmann
Copy link
Contributor Author

You are awesome @nhovratov, thanks for all the efforts :)

@DanielSiepmann DanielSiepmann changed the title WIP|POC:Out of the box grid rendering for backend WIP|POC:Out of the inline relation rendering for backend Jan 30, 2024
@nhovratov
Copy link
Owner

The backend preview feature for nested content elements is included in this commit: 2779792
Due to refactoring in the meantime the merge conflicts got too complex, so I had to implement this again outside of this PR.

Two things left to do:

I would say, let's open a new issue for that. This PR got a little messy over the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants