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

Adjust Feedback/form.phtml to allow overriding the default action route #4187

Merged
merged 6 commits into from
Jan 16, 2025

Conversation

LuomaJuha
Copy link
Contributor

No description provided.

@LuomaJuha
Copy link
Contributor Author

LuomaJuha commented Jan 16, 2025

hmm, some odd deprecations, nvm watched the wrong errors..

Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @LuomaJuha -- see below for a few questions.


$formUrl = $this->url('feedback-form', ['id' => $this->formId]);
$formUrl = $this->url($this->formActionUrl ?? 'feedback-form', ['id' => $this->formId]);
Copy link
Member

Choose a reason for hiding this comment

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

Where is formActionUrl set? Should this actually be $this->formActionUrl ?? $this->url(...) so you override the actual URL instead of the route name? If not, should the variable be named formActionRoute instead?

Copy link
Contributor Author

@LuomaJuha LuomaJuha Jan 16, 2025

Choose a reason for hiding this comment

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

hmm, actually setting the route in feedbackform yaml could be enough as everything else can be otbained from params and yaml, added a function for it now :) Gonna add a comment for it also

@LuomaJuha LuomaJuha requested a review from demiankatz January 16, 2025 14:08
Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks, @LuomaJuha!

@demiankatz demiankatz merged commit da72b24 into vufind-org:dev Jan 16, 2025
6 checks passed
@demiankatz demiankatz added this to the 11.0 milestone Jan 16, 2025
@demiankatz demiankatz changed the title Adjust Feedback/form.phtml to allow overriding the default action url Adjust Feedback/form.phtml to allow overriding the default action route Jan 16, 2025
@LuomaJuha LuomaJuha deleted the adjust-form-to-allow-new-url branch January 16, 2025 14:17
LuomaJuha added a commit to NatLibFi/NDL-VuFind2 that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants