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

!!! TASK: Make node label pure neos concept #5020

Merged

Conversation

mhsdesign
Copy link
Member

@mhsdesign mhsdesign commented Apr 30, 2024

Solves partially #5019

Upgrade instructions

To get the label for a Node in PHP, one must replace Node::getLabel() with a call to Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface::getLabel:

+  #[Flow\Inject]
+ protected NodeLabelGeneratorInterface $nodeLabelGenerator;
+ 
+ $this->nodeLabelGenerator->getLabel($node);
- $node->getLabel();

In Fusion accessing the label via ${node.label} will currently no longer work but one can leverage the flowquery ${q(node).label()} to get the label.

Additionally NodeType::getNodeLabelGenerator has been removed. Please inject the NodeLabelGeneratorInterface instead.

Review instructions

Followup node.label viewhelper for fluid (see also #5023)
Rector migrations will be provided as part of neos/rector#57

The Label concept was removed from the core as it requires by its current implementation that the NodeType is available in the Node. Though there are ways to work around this by adding a label to the nodes constructor via some factory, seeing that the label is not used once in the core made us realise that this is just an arbitrary convenience thing attached to the core. To better have Neos' decide what it wants and how implementation / extensibility and co should look like we remove this concern from the core to also ease standalone use-cases.

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@mhsdesign mhsdesign force-pushed the task/5019-make-node-label-pure-neos-concept branch from 3f51825 to 299a579 Compare April 30, 2024 11:30
Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Nice, I like!
Just a comment re asset vs node label and IMO we should make NodeLabelRenderer::getNodeLabelGeneratorForNodeType() private

@mhsdesign
Copy link
Member Author

The tests fail currently because Neos requires the Ui as well which is a bug: #4951 (comment)

But when the ui is adjusted everything will work: neos/neos-ui#3772

@mhsdesign mhsdesign force-pushed the task/5019-make-node-label-pure-neos-concept branch from 299a579 to edd67a6 Compare April 30, 2024 14:06
@mhsdesign mhsdesign changed the title TASK: Make node label pure neos concept !!! TASK: Make node label pure neos concept Apr 30, 2024
@mhsdesign
Copy link
Member Author

We discussed that we want rather a flowquery operation for this for simplicity.
Also i plan to make beta 10 not super super breaky but maybe even little forgiving so we can add this b/c and remove it with 11:
The whole Node read model will consist of 50% deprecated code then mostly already because of #5042 but thats fine imo this one time.

/**
 * Returned the node label as generated by the configured node label generator.
 *
 * In PHP please use Neos' {@see NodeLabelRenderer} instead.
 * 
 * For Fusion please use the FlowQuery operation:
 * ```
 * ${q(node).label()}
 * ```
 *
 * @deprecated will be removed before the final 9.0 release
 */
public function getLabel(): string
{
    // highly illegal
    return (new NodeLabelRenderer())->renderNodeLabel($this);
}

@mhsdesign mhsdesign force-pushed the task/5019-make-node-label-pure-neos-concept branch from 3c19d54 to 8e0f963 Compare May 13, 2024 12:37
@mhsdesign mhsdesign force-pushed the task/5019-make-node-label-pure-neos-concept branch from 8e0f963 to b687896 Compare May 13, 2024 12:42
mhsdesign added a commit to mhsdesign/neos-ui that referenced this pull request May 13, 2024
@mhsdesign mhsdesign requested a review from nezaniel May 13, 2024 15:23
@mhsdesign mhsdesign requested a review from kitsunet May 14, 2024 08:48
@kitsunet kitsunet enabled auto-merge May 14, 2024 08:49
@kitsunet kitsunet merged commit 5d14a98 into neos:9.0 May 14, 2024
5 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants