-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Adjust the implementation to the new Twig Hooks version #16173
Adjust the implementation to the new Twig Hooks version #16173
Conversation
Bunnyshell Preview Environment deletedAvailable commands:
|
66aea6d
to
de29f05
Compare
de29f05
to
563b1db
Compare
775e35d
to
30119a5
Compare
30119a5
to
f25a8bd
Compare
parentMainHook: '@=parent_main_hook' | ||
parentFallbackHook: '@=parent_fallback_hook' | ||
props: | ||
channel: '@=_context.resource' |
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.
We need to add the way of passing the resource to components/templates to our convention, now it's either the resource name or simply resource
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.
by the way the we will stick out to the resource name rather than simply resource
?
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.
With templates in mind we could just add both, that's basically in line with how those variables were in the templates before.
With components, it makes meagre sense since it needs to be added as a live prop (probably will be ignored otherwise, though I have not checked), in that case, I'd go w/ the resource name (stuff like public ?Product $resource = null;
etc. imo looks weird).
Either way we gotta talk it through and decide what's the better option.
props: | ||
form: '@=_context.form' | ||
resource: '@=_context.resource' | ||
isSimple: '@=_context.resource.isSimple()' |
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.
Having the property accessor take care of these expressions would be great, then we could go
@=context.resource.isSimple
or just
@=context.resource.simple
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.
I'll try to do it in Twig Hooks v0.3
.
src/Sylius/Bundle/AdminBundle/Resources/views/ProductVariant/_form.html.twig
Outdated
Show resolved
Hide resolved
sylius_test_form_attribute('update-changes-button')|merge({ | ||
text: 'sylius.ui.update'|trans, | ||
type: 'submit', | ||
form: hookable_metadata.context.form.vars.id | ||
}) |
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.
We really should reverse this merge, it looks weird having test attributes as the base.
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.
Or we should a better way to add test form attributes :D.
...s/Bundle/AdminBundle/Resources/views/Shared/Crud/Index/Content/Header/_breadcrumbs.html.twig
Outdated
Show resolved
Hide resolved
@@ -19,7 +16,7 @@ | |||
<div class="page-body"> | |||
<div class="container-xl"> | |||
{% block content %} | |||
{% set renderRest = hookable_configuration.render_rest %} | |||
{% set renderRest = hookable_metadata.configuration.render_rest %} |
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.
Maybe we should add a global switch in the configuration regarding render_rest
?
features/admin/promotion/managing_catalog_promotions/toggling_catalog_promotion.feature
Show resolved
Hide resolved
features/admin/promotion/managing_promotions/adding_promotion_with_filter.feature
Show resolved
Hide resolved
src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/common/component/navbar.yaml
Show resolved
Hide resolved
src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/common/component/navbar.yaml
Show resolved
Hide resolved
src/Sylius/Bundle/AdminBundle/Resources/config/app/twig_hooks/order/show.yaml
Show resolved
Hide resolved
src/Sylius/Bundle/AdminBundle/Resources/views/Order/Show/Content/_sections.html.twig
Show resolved
Hide resolved
26fda4a
to
60319eb
Compare
60319eb
to
d627142
Compare
062fb9d
into
Sylius:bootstrap-admin-panel
Thank you, @jakubtobiasz! |
No description provided.