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

[Feature] Allows row_attr on form rows #14693

Merged
merged 1 commit into from Jan 18, 2023
Merged

Conversation

Prometee
Copy link
Contributor

@Prometee Prometee commented Jan 9, 2023

Q A
Branch? 1.13
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets none
License MIT

This PR allows to use row_attr of each form_row which wasn't possible with the current code.

@Prometee Prometee requested a review from a team as a code owner January 9, 2023 12:22
@jakubtobiasz
Copy link
Member

Hi @Prometee!
Thanks for your work. Can you rebase your PR? We fixed CI, so build should work now :).

@jakubtobiasz jakubtobiasz added the Feature New feature proposals. label Jan 10, 2023
@Prometee Prometee changed the title [New Feature] Allows row_attr on form rows [Feature] Allows row_attr on form rows Jan 10, 2023
@Prometee
Copy link
Contributor Author

@jakubtobiasz sure ! It's done.

@@ -1,7 +1,14 @@
{% extends 'form_div_layout.html.twig' %}

{% block form_row -%}
<div class="{% if required %}required {% endif %}field{% if (not compound or force_error|default(false)) and not valid %} error{% endif %}">
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' field'}) %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' field'}) %}
{% set row_attr = row_attr|merge({'class': row_attr.class|default ~ ' field'}) %}

<div class="{% if required %}required {% endif %}field{% if (not compound or force_error|default(false)) and not valid %} error{% endif %}">
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' field'}) %}
{% if required %}
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' required'}) %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' required'}) %}
{% set row_attr = row_attr|merge({'class': row_attr.class|default ~ ' required'}) %}

{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' required'}) %}
{% endif %}
{% if (not compound or force_error|default(false)) and not valid %}
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' error'}) %}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{% set row_attr=row_attr|merge({'class': row_attr.class|default ~ ' error'}) %}
{% set row_attr = row_attr|merge({'class': row_attr.class|default ~ ' error'}) %}

@GSadee GSadee merged commit 74bdaea into Sylius:1.13 Jan 18, 2023
@GSadee
Copy link
Member

GSadee commented Jan 18, 2023

Thank you, Francis! 🎉

@Prometee Prometee deleted the row-attr branch April 21, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature proposals.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants