10000 [TwigBridge] Improve form_errors of bootstrap5 form theme by ttskch · Pull Request #49243 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[TwigBridge] Improve form_errors of bootstrap5 form theme #49243

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
Merged
merged 1 commit into from
Feb 6, 2023
Merged

[TwigBridge] Improve form_errors of bootstrap5 form theme #49243

merged 1 commit into from
Feb 6, 2023

Conversation

ttskch
Copy link
Contributor
@ttskch ttskch commented Feb 5, 2023 •
edited
Loading
Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

This PR improves output of form_errors() for the root form with Bootstrap 5 form theme.

Example

Usage code 
public function buildForm(FormBuilderInterface $builder, array $options): void
{
    $builder
        ->add('text', TextType::class, [
            'constraints' => new Assert\NotBlank(),
        ])
        ->add('textarea', TextareaType::class, [
            'attr' => [
                'rows' => 3,
            ],
            'constraints' => new Assert\NotBlank(),
        ]
8000
)
        ->add('select', ChoiceType::class, [
            'choices' => [
                'Alice' => 'Alice',
                'Bob' => 'Bob',
                'Charlie' => 'Charlie',
            ],
            'constraints' => new Assert\Choice(choices: ['Dave']),
        ])
        ->add('selectMultiple', ChoiceType::class, [
            'multiple' => true,
            'choices' => [
                'Alice' => 'Alice',
                'Bob' => 'Bob',
                'Charlie' => 'Charlie',
            ],
            'constraints' => new Assert\Choice(choices: ['Dave']),
        ])
        ->add('radio', ChoiceType::class, [
            'expanded' => true,
            'choices' => [
                'Alice' => 'Alice',
                'Bob' => 'Bob',
                'Charlie' => 'Charlie',
            ],
            'constraints' => new Assert\Choice(choices: ['Dave']),
        ])
        ->add('checkbox', ChoiceType::class, [
            'expanded' => true,
            'multiple' => true,
            'choices' => [
                'Alice' => 'Alice',
                'Bob' => 'Bob',
                'Charlie' => 'Charlie',
            ],
            'constraints' => new Assert\Choice(choices: ['Dave']),
        ])
        ->add('singleCheckbox', CheckboxType::class, [
            'constraints' => new Assert\NotBlank(),
        ])
    ;
}

Before

image

After

image

@carsonbot carsonbot added this to the 5.4 milestone Feb 5, 2023
@xabbuh
Copy link
Member
xabbuh commented Feb 5, 2023

Can you please update the screenshots to also include a ChoiceType with expanded set to true? Reason is that in this case the ChoiceType is also compound and I wonder if the layout would still look right then.

@ttskch ttskch force-pushed the feat/twig-bridge-bootstrap5-form-theme branch from 95519e0 to db2e616 Compare February 5, 2023 09:35
@fabpot
Copy link
Member
fabpot commented Feb 5, 2023

It looks like something for 6.3 instead.

@ttskch ttskch force-pushed the feat/twig-bridge-bootstrap5-form-theme branch from db2e616 to f78d5d1 Compare February 5, 2023 09:42
@ttskch ttskch changed the base branch from 5.4 to 6.3 February 5, 2023 09:43
@ttskch
Copy link
Contributor Author
ttskch commented Feb 5, 2023 •
edited
Loading

@xabbuh Thanks for your advice! You are right, there was a problem with my earlier code. I fixed it and re-pushed and also attached the complete usage code and screenshots to the description. Now I'm working on fixing some tests.

refs
https://github.com/symfony/symfony/blob/6.3/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig#L296

@ttskch
Copy link
Contributor Author
ttskch commented Feb 5, 2023 •
edited
Loading

@fabpot OK, I've just rebased.

@ttskch
Copy link
Contributor Author
ttskch commented Feb 5, 2023 •
edited
Loading

I've fixed some tests for this PR. continuous-integration/appveyor/pr was originally failed on the base 6.3 branch and is not fixed in this PR🤔

@OskarStark OskarStark modified the milestones: 5.4, 6.3 Feb 6, 2023
@OskarStark OskarStark added Feature and removed Bug labels Feb 6, 2023
javiereguiluz
javiereguiluz approved these changes Feb 6, 2023
View reviewed changes
Copy link
Member
@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

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

This is a nice improvement!

Takashi, thanks for your contribution.

@nicolas-grekas nicolas-grekas force-pushed the feat/twig-bridge-bootstrap5-form-theme branch from ecfc662 to f4c546d Compare February 6, 2023 09:34
@nicolas-grekas
Copy link
Member

Thank you @ttskch.

@nicolas-grekas nicolas-grekas merged commit d0ef4f0 into symfony:6.3 Feb 6, 2023
@ttskch ttskch deleted the feat/twig-bridge-bootstrap5-form-theme branch February 6, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javiereguiluz javiereguiluz javiereguiluz approved these changes

@xabbuh xabbuh xabbuh approved these changes

@yceruto yceruto yceruto approved these changes

@lyrixx lyrixx Awaiting requested review from lyrixx

@wouterj wouterj Awaiting requested review from wouterj

@chalasr chalasr Awaiting requested review from chalasr

@dunglas dunglas Awaiting requested review from dunglas

@OskarStark OskarStark Awaiting requested review from OskarStark

Assignees
No one assigned
Labels
Feature Status: Reviewed TwigBridge
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
8 participants
@ttskch @xabbuh @fabpot @nicolas-grekas @javiereguiluz @yceruto @OskarStark @carsonbot
0