Skip to content

[Nette] forms setRequired(false)#28

Merged
TomasVotruba merged 22 commits intomasterfrom
nette-forms-set-required
Sep 6, 2017
Merged

[Nette] forms setRequired(false)#28
TomasVotruba merged 22 commits intomasterfrom
nette-forms-set-required

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

@TomasVotruba TomasVotruba commented Sep 5, 2017

Closes #27

image

What is the function?

I really didn't get it from the forum, so at the moment it works like this:

Find on variable of Nette\Application\UI\Form type:

->addCondition($form::FILLED)

and replace it by:

->setRequired(false)

Could you confirm/help me with that @dg && @petrvacha?

@TomasVotruba TomasVotruba merged commit 645d92d into master Sep 6, 2017
@TomasVotruba TomasVotruba deleted the nette-forms-set-required branch September 6, 2017 22:33
@dg
Copy link
Copy Markdown

dg commented Sep 9, 2017

Just note: in specific cases, this may be an incompatible change.

@TomasVotruba
Copy link
Copy Markdown
Member Author

I don't actually understand it... this is just first example.

Could you provide cases to make it compatible?

@dg
Copy link
Copy Markdown

dg commented Sep 9, 2017

I mean rare cases. For example:

$form->addText('name')
    ->addCondition($form::FILLED)
        ->addRule(...)
        ->addRule(...);

$form['name']
        ->addRule(...); // this rule is called regardless of whether it is filled in (Nette 2.4 triggers warning here that will force you to decide whether the field is required or not)

Automatically changed code:

$form->addText('name')
    ->setRequired(false)
    ->addRule(...)
    ->addRule(...);

$form['name']
        ->addRule(...); // now is this rule called only when input is filled (no warning is triggered)

@TomasVotruba
Copy link
Copy Markdown
Member Author

Thanks. I haven't seen such use case yet.

So there should be added another refactoring, that adds setRequired() if there is only addRule()?

@dg
Copy link
Copy Markdown

dg commented Sep 9, 2017

Yes, when there is addRule(), there must be setRequired().

But every such changes must be checked by the programmer, because in some cases it can change the function.

@TomasVotruba
Copy link
Copy Markdown
Member Author

Sure, the aim is to keep programmer out of manually work and keep only checking. Something like code review.

Thanks, I will look on that later

TomasVotruba added a commit that referenced this pull request May 14, 2021
rectorphp/rector-src@15be298 [Config] update duplicate Skip fixture config (#28)
echo511 pushed a commit to echo511/rector that referenced this pull request Sep 4, 2021
* [Config] Remove duplicate Skip fixture config

* uupdate to */Fixture*/*
@ebreeze ebreeze mentioned this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants