Skip to content

fix: keep multi-choice visibleJs expressions attribute-safe and id-typed #178

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

Merged
ManukMinasyan merged 3 commits into from
Jul 1, 2026
Merged

fix: keep multi-choice visibleJs expressions attribute-safe and id-typed #178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12ec66e
fix: keep multi-choice visibleJs expressions attribute-safe and id-typed
ManukMinasyan Jul 1, 2026
135bdba
style: use sprintf for generated JS expressions (rector)
ManukMinasyan Jul 1, 2026
cce6801
test: cover multi-choice contains for section visibility expression
ManukMinasyan Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 41 additions & 36 deletions src/Services/Visibility/FrontendVisibilityService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,13 @@ private function buildCondition(
$escapedCode = addslashes($condition->field_code);

$targetField = $isModelAttribute ? null : $allFields?->firstWhere('code', $condition->field_code);

// Option resolution (name -> id) and the option-backed choice branch both depend on the
// target field's options being loaded. Callers do not always eager-load them, so guarantee
// it here — otherwise a choice condition falls back to comparing option names against the
// selected ids, which never matches and emits quoted strings that break the class binding.
$targetField?->loadMissing('options');
Comment on lines +227 to +231

$fieldValue = $isModelAttribute
? sprintf("\$get('%s')", $escapedCode)
: sprintf("\$get('custom_fields.%s')", $escapedCode);
Expand Down Expand Up @@ -422,23 +429,21 @@ private function buildOptionExpression(
}

/**
* Build multi-value option condition.
* Build multi-value option condition as a single-line expression (no block-body arrow, no double
* quotes) so it embeds safely inside Filament's `x-bind:class="{ 'fi-hidden': !(…) }"` attribute.
* Both sides are stringified before comparison because option ids arrive from Livewire state as
* strings while the resolved condition ids are integers — strict includes() would otherwise miss.
*/
private function buildMultiValueOptionCondition(
string $fieldValue,
mixed $resolvedValue,
string $jsValue
): string {
$selected = sprintf('(Array.isArray(%s) ? %s : []).map(v => String(v))', $fieldValue, $fieldValue);

return is_array($resolvedValue)
? "(() => {
const fieldVal = Array.isArray({$fieldValue}) ? {$fieldValue} : [];
const conditionVal = {$jsValue};
return conditionVal.some(id => fieldVal.includes(id));
})()"
: "(() => {
const fieldVal = Array.isArray({$fieldValue}) ? {$fieldValue} : [];
return fieldVal.includes({$jsValue});
})()";
? sprintf('(%s.map(v => String(v)).some(id => %s.includes(id)))', $jsValue, $selected)
: sprintf('(%s.includes(String(%s)))', $selected, $jsValue);
}

/**
Expand All @@ -448,24 +453,12 @@ private function buildSingleValueOptionCondition(
string $fieldValue,
string $jsValue
): string {
return "(() => {
const fieldVal = {$fieldValue};
const conditionVal = {$jsValue};

if (fieldVal === null || fieldVal === undefined || fieldVal === '') {
return conditionVal === null || conditionVal === undefined || conditionVal === '';
}

if (typeof fieldVal === 'number' && typeof conditionVal === 'number') {
return fieldVal === conditionVal;
}

if (typeof fieldVal === 'boolean' && typeof conditionVal === 'boolean') {
return fieldVal === conditionVal;
}
$fieldEmpty = sprintf("(%s === null || %s === undefined || %s === '')", $fieldValue, $fieldValue, $fieldValue);
$conditionEmpty = sprintf("(%s === null || %s === undefined || %s === '')", $jsValue, $jsValue, $jsValue);

return String(fieldVal) === String(conditionVal);
})()";
// Single-line, string-compared (String() subsumes the number/boolean cases) so it stays safe
// inside Filament's double-quoted x-bind:class attribute.
return sprintf('(%s ? %s : String(%s) === String(%s))', $fieldEmpty, $conditionEmpty, $fieldValue, $jsValue);
}

/**
Expand Down Expand Up @@ -564,13 +557,9 @@ private function buildContainsExpression(
: $value;
$jsValue = $this->formatJsValue($resolvedValue);

return "(() => {
const fieldVal = {$fieldValue};
const searchVal = {$jsValue};
return Array.isArray(fieldVal)
? fieldVal.some(item => String(item).toLowerCase().includes(String(searchVal).toLowerCase()))
: String(fieldVal || '').toLowerCase().includes(String(searchVal).toLowerCase());
})()";
return sprintf('(Array.isArray(%s) ', $fieldValue).
sprintf('? %s.some(item => String(item).toLowerCase().includes(String(%s).toLowerCase())) ', $fieldValue, $jsValue).
sprintf(": String(%s || '').toLowerCase().includes(String(%s).toLowerCase()))", $fieldValue, $jsValue);
}

/**
Expand Down Expand Up @@ -613,7 +602,7 @@ private function formatJsValue(mixed $value): string
is_bool($value) => $value ? 'true' : 'false',
$value === 'true' => 'true',
$value === 'false' => 'false',
is_string($value) => json_encode($value, JSON_UNESCAPED_UNICODE),
is_string($value) => $this->toJsString($value),
is_int($value) => (string) $value,
is_float($value) => number_format($value, 10, '.', ''),
is_numeric($value) => str_contains($value, '.')
Expand All @@ -626,10 +615,26 @@ private function formatJsValue(mixed $value): string
$collection->implode(', ').
']'
),
default => json_encode((string) $value, JSON_UNESCAPED_UNICODE),
default => $this->toJsString((string) $value),
};
}

/**
* Emit a single-quoted JS string literal. Single quotes (never double) keep the expression safe
* inside Filament's double-quoted `x-bind:class="…"` attribute, and control characters are
* stripped so the generated visibleJs never spans multiple lines or breaks Alpine parsing.
*/
private function toJsString(string $value): string
{
$escaped = str_replace(
['\\', "'", "\r", "\n", "\t"],
['\\\\', "\\'", '', ' ', ' '],
$value,
);
Comment on lines +629 to +633

return sprintf("'%s'", $escaped);
}

/**
* Export visibility logic to JavaScript format for complex integrations.
*
Expand Down
49 changes: 48 additions & 1 deletion tests/Feature/SectionVisibilityIntegrationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Relaticle\CustomFields\Facades\CustomFields;
use Relaticle\CustomFields\FeatureSystem\FeatureConfigurator;
use Relaticle\CustomFields\Models\CustomField;
use Relaticle\CustomFields\Models\CustomFieldOption;
use Relaticle\CustomFields\Models\CustomFieldSection;
use Relaticle\CustomFields\Services\Visibility\BackendVisibilityService;
use Relaticle\CustomFields\Services\Visibility\CoreVisibilityLogicService;
Expand Down Expand Up @@ -134,7 +135,53 @@

expect($jsExpression)->toBeString()
->and($jsExpression)->toContain("\$get('custom_fields.plan')")
->and($jsExpression)->toContain('"pro"');
->and($jsExpression)->toContain("'pro'")
->and($jsExpression)->not->toContain('"');
});

it('generates an attribute-safe, id-based expression for a multi-choice contains section condition', function (): void {
$services = CustomField::factory()->create([
'name' => 'Services',
'code' => 'services',
'type' => 'multi-select',
'entity_type' => Post::class,
]);
$other = CustomFieldOption::factory()->create([
'custom_field_id' => $services->id,
'name' => 'Other',
'sort_order' => 1,
]);

$section = CustomFieldSection::factory()->create([
'name' => 'Multi Choice Section',
'entity_type' => Post::class,
'active' => true,
'settings' => [
'visibility' => [
'mode' => VisibilityMode::SHOW_WHEN,
'logic' => VisibilityLogic::ALL,
'conditions' => [
[
'field_code' => 'services',
'operator' => VisibilityOperator::CONTAINS,
'value' => ['Other'],
'source' => ConditionSource::CustomField,
],
],
],
],
]);

$jsExpression = $this->frontendService->buildSectionVisibilityExpression($section, collect([$services]));

// Same guarantees as field-level conditions: resolves the option name to its numeric id,
// string-normalizes, and stays safe inside the double-quoted x-bind:class attribute.
expect($jsExpression)->toBeString()
->and($jsExpression)->toContain((string) $other->id)
->and($jsExpression)->not->toContain("'Other'")
->and($jsExpression)->toContain('String(v)')
->and($jsExpression)->not->toContain('"')
->and($jsExpression)->not->toContain("\n");
});

it('generates JS expression for section with model attribute condition', function (): void {
Expand Down
54 changes: 53 additions & 1 deletion tests/Feature/UnifiedVisibilityConsistencyTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Relaticle\CustomFields\FieldTypeSystem\BaseFieldType;
use Relaticle\CustomFields\FieldTypeSystem\FieldSchema;
use Relaticle\CustomFields\Models\CustomField;
use Relaticle\CustomFields\Models\CustomFieldOption;
use Relaticle\CustomFields\Models\CustomFieldSection;
use Relaticle\CustomFields\Services\Visibility\BackendVisibilityService;
use Relaticle\CustomFields\Services\Visibility\CoreVisibilityLogicService;
Expand Down Expand Up @@ -183,7 +184,11 @@

expect($jsExpression)->toBeString()
->and($jsExpression)->toContain("\$get('custom_fields.status')")
->and($jsExpression)->toContain('"active"');
->and($jsExpression)->toContain("'active'")
// Must embed safely inside Filament's double-quoted x-bind:class attribute: never emit
// double quotes (they truncate the attribute) and stay on a single line.
->and($jsExpression)->not->toContain('"')
->and($jsExpression)->not->toContain("\n");

// Test always visible field returns null (no expression needed)
$alwaysVisibleExpression = $this->frontendService->buildVisibilityExpression($this->alwaysVisibleField, $fields);
Expand All @@ -198,6 +203,53 @@
->and($jsData['fields']['name']['has_visibility_conditions'])->toBeFalse();
});

test('multi-choice contains expression is attribute-safe, id-based and string-normalized', function (): void {
$services = CustomField::factory()->create([
'custom_field_section_id' => $this->section->id,
'name' => 'Services',
'code' => 'services',
'type' => 'multi-select',
]);
$other = CustomFieldOption::factory()->create([
'custom_field_id' => $services->id,
'name' => 'Other',
'sort_order' => 1,
]);

$dependent = CustomField::factory()->create([
'custom_field_section_id' => $this->section->id,
'name' => 'Other details',
'code' => 'other_details',
'type' => 'text',
'settings' => [
'visibility' => [
'mode' => VisibilityMode::SHOW_WHEN,
'logic' => VisibilityLogic::ALL,
'conditions' => [[
'field_code' => 'services',
'operator' => VisibilityOperator::CONTAINS,
'value' => ['Other'],
]],
'always_save' => false,
],
],
]);

$expr = $this->frontendService->buildVisibilityExpression($dependent, collect([$services, $dependent]));

expect($expr)->toBeString()
// The option name is resolved to its numeric id — the expression never compares the
// selected ids against the raw option name (which would never match).
->and($expr)->toContain((string) $other->id)
->and($expr)->not->toContain("'Other'")
// Multi-select ids arrive from Livewire state as strings while the resolved condition id
// is an int; both sides must be stringified or strict includes() silently misses.
->and($expr)->toContain('String(v)')
// Must embed safely inside Filament's double-quoted x-bind:class attribute.
->and($expr)->not->toContain('"')
->and($expr)->not->toContain("\n");
});

test('complex conditions work identically in backend and frontend', function (): void {
// Create a more complex scenario with nested dependencies
$dependentField = CustomField::factory()->create([
Expand Down