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

Autoinject all admin variables #8116

Merged
merged 1 commit into from Oct 11, 2023

Conversation

core23
Copy link
Member

@core23 core23 commented Oct 6, 2023

Subject

I am targeting this branch, because this feature is BC.

Closes #7242

Changelog

### Changed
- Automatically detect `base_template` and `admin` twig variable

### Deprecated
- Deprecate `CRUDController::renderWithExtraParams` method
- Deprecate `CRUDController::addRenderExtraParams` method
- Deprecate `CRUDController::getBaseTemplate` method

@core23 core23 added this to the 4.0 milestone Oct 6, 2023
@core23 core23 force-pushed the twig-admin-variables branch 4 times, most recently from da978d5 to 4984fc0 Compare October 6, 2023 22:43
Copy link
Member

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

Deprecated method should be added to the changelog

@@ -135,6 +135,9 @@ public function listAction(Request $request): Response
$exportFormats = $exporter->getAvailableFormats($this->admin);
}

/**
* @psalm-suppress DeprecatedMethod
Copy link
Member

Choose a reason for hiding this comment

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

A NEXT_MAJOR replace with render() should be added (?)

Copy link
Member Author

Choose a reason for hiding this comment

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

The NEXT_MAJOR comment is pedantic because the annotation should provide enough information

Copy link
Member

Choose a reason for hiding this comment

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

When we had to resolve all the 4.x NEXT_MAJOR some of them were not obvious, so there is never too much comment.

@psalm-suppress just tell to ignore the deprecation but does not explain how to solve it.

At least

NEXT_MAJOR: Remplace this method by `$this->render`

above renderWithExtraParams definition might be more useful than remove this method.

src/EventListener/AdminEventListener.php Show resolved Hide resolved
@jordisala1991
Copy link
Member

This method should be depreacted too:

protected function getBaseTemplate(): string

@core23 core23 force-pushed the twig-admin-variables branch 3 times, most recently from 3916094 to 8f51141 Compare October 10, 2023 15:27
@core23
Copy link
Member Author

core23 commented Oct 10, 2023

Please review again

@core23 core23 merged commit 2a65e55 into sonata-project:4.x Oct 11, 2023
24 checks passed
@core23 core23 deleted the twig-admin-variables branch October 11, 2023 07:45
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.

Extract the way we choose base_template for actions
3 participants