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.

Sign up for GitHub

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

Jump to bottom

[Turbo] Add <Turbo:Frame> component #2303

Merged
merged 1 commit into from
Oct 30, 2024
Merged

[Turbo] Add <Turbo:Frame> component #2303

merged 1 commit into from
Oct 30, 2024

Conversation

seb-jean
Copy link
Contributor

@seb-jean seb-jean commented Oct 25, 2024 •
edited
Loading

Q A
Bug fix? no
New feature? yes
License MIT

I added a <twig:Turbo:Frame> component.

Simple example

<twig:Turbo:Frame id="the_frame_id" />

{# renders as: #}
<turbo-frame id="the_frame_id"></turbo-frame>

With a HTML attributes

<twig:Turbo:Frame id="the_frame_id" loading="lazy" src="{{ path('block') }}" />

{# renders as: #}
<turbo-frame id="the_frame_id" loading="lazy" src="https://example.com/block"></turbo-frame>

With content

<twig:Turbo:Frame id="the_frame_id" src="{{ path('block') }}">
    A placeholder.
</twig:Turbo:Frame>

{# renders as: #}
<turbo-frame id="the_frame_id" src="https://example.com/block">A placeholder.
</turbo-frame>

@carsonbot carsonbot added Feature New Feature Turbo Status: Needs Review Needs to be reviewed labels Oct 25, 2024
Kocal
Kocal approved these changes Oct 26, 2024
View reviewed changes
Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestion!
Can you update the CHANGELOG please?

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Oct 26, 2024
smnandre
smnandre requested changes Oct 27, 2024
View reviewed changes
Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

Little change needed in the component template

@carsonbot carsonbot added Status: Needs Work Additional work is needed Status: Needs Review Needs to be reviewed and removed Status: Reviewed Has been reviewed by a maintainer Status: Needs Work Additional work is needed labels Oct 27, 2024
@seb-jean seb-jean mentioned this pull request Oct 28, 2024
Merged
smnandre
smnandre approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

Thank you!

Now we need to document this new component!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Oct 28, 2024
@smnandre smnandre mentioned this pull request Oct 30, 2024
Open
@seb-jean
Copy link
Contributor Author

Thank you!

Now we need to document this new component!

Done

smnandre
smnandre reviewed Oct 30, 2024
View reviewed changes
src/Turbo/doc/index.rst Outdated Show resolved Hide resolved
smnandre
smnandre reviewed Oct 30, 2024
View reviewed changes
src/Turbo/doc/index.rst Outdated Show resolved Hide resolved
@Kocal
Copy link
Member

Kocal commented Oct 30, 2024

PR rebased, conflicts fixed, and @smnandre comments applied 🫶

@Kocal
Copy link
Member

Kocal commented Oct 30, 2024

Thank you @seb-jean.

@Kocal Kocal merged commit ed3dc44 into symfony:2.x Oct 30, 2024
12 of 13 checks passed
@seb-jean
Copy link
Contributor Author

Thank you @seb-jean.

With pleasure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smnandre smnandre smnandre approved these changes

@Kocal Kocal Kocal approved these changes

Assignees
No one assigned
Labels
Feature New Feature Status: Reviewed Has been reviewed by a maintainer Turbo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@seb-jean @Kocal @smnandre @carsonbot