Conversation
|
I think normally we don't add translations in a PR, they are managed via Transifex through a separate process (I believe). Should those be removed from the PR, @GuySartorelli? |
|
Correct, thanks @michalkleiner. |
|
Also you can add the |
| if ($this->data()->ConfirmationPageID) { | ||
| $confirmationPage = $this->data()->ConfirmationPage(); | ||
| if ($confirmationPage && $confirmationPage->exists()) { |
There was a problem hiding this comment.
| if ($this->data()->ConfirmationPageID) { | |
| $confirmationPage = $this->data()->ConfirmationPage(); | |
| if ($confirmationPage && $confirmationPage->exists()) { | |
| $confirmationPage = $this->data()->ConfirmationPage(); | |
| if ($confirmationPage && $confirmationPage->exists()) { |
This is a simpler way to do the condition - though not by any means necessary for merging.
| /** | ||
| * @var array | ||
| */ | ||
| private static $has_one = [ |
There was a problem hiding this comment.
| /** | |
| * @var array | |
| */ | |
| private static $has_one = [ | |
| private static array $has_one = [ |
New API should be strongly typed instead of relying on PHPDocs.
| ), | ||
| TreeDropdownField::create( | ||
| 'ConfirmationPageID', | ||
| _t('SilverStripe\\UserForms\\Model\\UserDefinedForm.CONFIRMATIONPAGE', 'Custom confirmation page'), | ||
| SiteTree::class | ||
| ), |
There was a problem hiding this comment.
This formfield should already be scaffolded as per SiteTree::scaffoldFormFieldForHasOne(). Is there a need to redeclare it here?
There was a problem hiding this comment.
Doing so allows the use of a new translation key.
There might be other ways of achieveing this that the author is unaware of.
Description
Add a new option to the Userform configuration: Custom confirmation page.
If this field contains a valid PageID from the SiteTree, when a form is successfully submitted in the frontend, the browser is redirected to the given page.
This allows form confirmation pages to be built individually, using all available content blocks (on an elemental page), allowing more flexibility/variety.
Manual testing steps
If there's no validation errors, you will be redirected to the selected confirmation page.
Issues
Pull request checklist
Note: Commit message is missing the ENH prefix. This was discovered after the change was already pushed. Changing the commit message (using rebase) will probably do more harm than good, so we decided against it. Please check if this is a show stopper for this PR