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

[5.x] Bring back select modifier #10219

Merged
merged 4 commits into from
Aug 1, 2024
Merged

[5.x] Bring back select modifier #10219

merged 4 commits into from
Aug 1, 2024

Conversation

jasonvarga
Copy link
Member

@jasonvarga jasonvarga commented May 29, 2024

This brings back the select modifier from #10183 which got reverted in #10218 because of #10211.

Except this will also fix the issue.

Todo:

  • Add test that shows the bug
  • Fix bug

@jasonvarga
Copy link
Member Author

jasonvarga commented May 29, 2024

@JohnathonKoster This one is probably for you 😄

I've tracked it down to here:

foreach ($recursiveParent->parameters as $param) {
if (ModifierManager::isModifier($param)) {
$childDataToUse = $this->runModifier($param->name, $parentParameterValues, $childDataToUse, $rootData);
}
}

When you use {{ *recursive children* }}, it ends up looking at the outer nav tag, seeing the select="" parameter, deciding that yes select is (now) a modifier, and then tries to run the modifier.

I believe this works this way so you can use scope as per this:

public function recursive_children_with_scope()
{
// the variables are inside RecursiveChildren@index
$this->app['statamic.tags']['recursive_children'] = RecursiveChildren::class;
$template = '<ul>{{ recursive_children scope="item" }}<li>{{ item:title }}.{{ item:foo }}.{{ foo }}{{ if item:children }}<ul>{{ *recursive item:children* }}</ul>{{ /if }}</li>{{ /recursive_children }}</ul>';
$expected = '<ul><li>One..Bar<ul><li>Two..Bar</li><li>Three..Bar<ul><li>Four.Baz.Baz</li></ul></li></ul></li></ul>';
$this->assertEquals($expected, $this->renderString($template, ['foo' => 'Bar'], true));
}

Making this change fixes it and lets scope continue to work, but obviously just scope. I can't really think of if that's a problem or not.

-if (ModifierManager::isModifier($param)) {
+if ($param->name === 'scope') {

JohnathonKoster and others added 2 commits May 30, 2024 19:12
TODO comment removed to address in separate PR
# Conflicts:
#	tests/Tags/StructureTagTest.php
@jasonvarga jasonvarga marked this pull request as ready for review August 1, 2024 13:32
@jasonvarga jasonvarga merged commit 2d7bc9f into 5.x Aug 1, 2024
17 checks passed
@jasonvarga jasonvarga deleted the bring-back-select-modifier branch August 1, 2024 13:33
duncanmcclean added a commit to statamic/docs that referenced this pull request Aug 6, 2024
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