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

Switch partial templates to anonymous Twig components #1426

Closed
wants to merge 2 commits into from

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Jul 24, 2023

This PR proposed to switch to the HTML syntax for components. It also introduce a new PostComponent component

@ker0x ker0x force-pushed the feature/component-html-syntax branch from 509bae1 to b3d8913 Compare July 24, 2023 00:44
@javiereguiluz
Copy link
Member

Thanks for this proposal. Before continuing working on this PR, let's ask JS/UX experts like @weaverryan, @kbond and @stof about this.

My non-expert opinion is divided. I think it's very obvious that making a UX component for search is correct ... but making an "empty" UX component just to display the post contents looks a bit "unnatural" for a Symfony app. It could confuse users about what's the traditional/common way of working in Symfony apps to display entity contents.

Let's wait for more opinions. Thanks!

@kbond
Copy link
Member

kbond commented Jul 24, 2023

FYI, once symfony/ux#802 is available, the empty component class will no longer be required.

@ker0x ker0x changed the title Add PostComponent and switch to component HTML syntax Switch partial templates to anonymous Twig components Sep 8, 2023
@ker0x ker0x force-pushed the feature/component-html-syntax branch from c012300 to f7fdf0a Compare September 8, 2023 11:49
@ker0x ker0x force-pushed the feature/component-html-syntax branch from f7fdf0a to 55a0c7e Compare September 8, 2023 11:53
@ker0x
Copy link
Contributor Author

ker0x commented Sep 8, 2023

@javiereguiluz, @kbond I've updated my PR to convert all partial templates into anonymous Twig components!

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.

5 participants