Skip to content

pr: [Nightly Fix] - Bug - Fix Alternate Row Color Guard#2

Open
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/alternate-color-check
Open

pr: [Nightly Fix] - Bug - Fix Alternate Row Color Guard#2
jewel-claw wants to merge 1 commit intomasterfrom
nightly-fix/alternate-color-check

Conversation

@jewel-claw
Copy link

What

  • fix the alternate row color condition in the public table CSS template

Why

  • the template checked alternate_color_status on $css_prefix, which is a selector string, not the colors array
  • on PHP 8 this can raise invalid offset warnings and it prevents zebra striping from honoring the saved setting

Fix

  • read alternate_color_status from $colors, which is the actual settings array used by the surrounding template

Confidence

  • linted app/Views/public/ninja-footable-css.php with php -l

@greptile-apps
Copy link

greptile-apps bot commented Mar 10, 2026

PR author is not in the allowed authors list.

@qodo-code-review
Copy link

Review Summary by Qodo

Fix alternate row color guard checking wrong variable

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fixed alternate row color condition checking wrong variable
• Changed from checking $css_prefix string to $colors array
• Prevents invalid offset warnings on PHP 8
• Restores zebra striping functionality with saved settings
Diagram
flowchart LR
  A["Incorrect: css_prefix string"] -->|Bug| B["Invalid offset warning"]
  C["Correct: colors array"] -->|Fix| D["Proper zebra striping"]
Loading

Grey Divider

File Changes

1. app/Views/public/ninja-footable-css.php 🐞 Bug fix +1/-1

Fix alternate color status variable reference

• Changed condition from isset($css_prefix['alternate_color_status']) to
 isset($colors['alternate_color_status'])
• Fixed variable reference to check the actual settings array instead of CSS selector string
• Ensures alternate row coloring respects the saved configuration setting

app/Views/public/ninja-footable-css.php


Grey Divider

Qodo Logo

@qodo-code-review
Copy link

qodo-code-review bot commented Mar 10, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. $colors offset on false 🐞 Bug ⛯ Reliability
Description
In ninja-footable-css.php the stackable zebra-striping condition accesses
$colors['alternate_color_status'] even when $colors can be boolean false, which can trigger
PHP warnings during CSS generation for stackable tables without custom colors enabled.
Code

app/Views/public/ninja-footable-css.php[221]

+    <?php if(isset($colors['alternate_color_status']) && $colors['alternate_color_status'] == 'yes'): ?>
Evidence
$hasStackable is enabled independently of color customization, while $colors defaults to false
unless Pro + table_color_type == custom_color. The CSS view is still included when
fonts/custom_css/cellStyles exist (common), and the stackable block evaluates
isset($colors['alternate_color_status']) outside the if ($colors) block, so $colors may be
non-array when this condition runs.

app/Views/public/ninja-footable-css.php[180-236]
app/Modules/DataProviders/NinjaFooTable.php[174-179]
app/Modules/DataProviders/NinjaFooTable.php[199-241]
app/Modules/DataProviders/NinjaFooTable.php[245-250]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The stackable-table zebra-striping guard evaluates `$colors[&#x27;alternate_color_status&#x27;]` outside the stackable section’s `if ($colors)` guard. Since `$colors` is initialized to `false` unless custom colors are enabled, this can lead to array-offset-on-bool warnings during CSS generation.

### Issue Context
`$hasStackable` can be enabled independently of custom colors. The CSS view is included even when `$colors` is false (e.g., when fonts/custom_css exist). The stackable section should only read `$colors[...]` when `$colors` is an array.

### Fix Focus Areas
- app/Views/public/ninja-footable-css.php[180-236]
- app/Views/public/ninja-footable-css.php[221-236]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@kilo-code-bot
Copy link

kilo-code-bot bot commented Mar 10, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (1 file)
  • app/Views/public/ninja-footable-css.php - Valid bug fix

Change Analysis

The fix correctly changes $css_prefix['alternate_color_status'] to $colors['alternate_color_status'] on line 221. This was a bug where:

  • $css_prefix is a string (CSS class prefix), not an array
  • Accessing $css_prefix['alternate_color_status'] would not work as intended in PHP
  • The alternate row color feature was likely not functioning before this fix

The fix is consistent with the rest of the file where all color settings correctly use the $colors array.

}
<?php endif; ?>
<?php if(isset($css_prefix['alternate_color_status']) && $css_prefix['alternate_color_status'] == 'yes'): ?>
<?php if(isset($colors['alternate_color_status']) && $colors['alternate_color_status'] == 'yes'): ?>

Choose a reason for hiding this comment

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

Action required

1. $colors offset on false 🐞 Bug ⛯ Reliability

In ninja-footable-css.php the stackable zebra-striping condition accesses
$colors['alternate_color_status'] even when $colors can be boolean false, which can trigger
PHP warnings during CSS generation for stackable tables without custom colors enabled.
Agent Prompt
### Issue description
The stackable-table zebra-striping guard evaluates `$colors['alternate_color_status']` outside the stackable section’s `if ($colors)` guard. Since `$colors` is initialized to `false` unless custom colors are enabled, this can lead to array-offset-on-bool warnings during CSS generation.

### Issue Context
`$hasStackable` can be enabled independently of custom colors. The CSS view is included even when `$colors` is false (e.g., when fonts/custom_css exist). The stackable section should only read `$colors[...]` when `$colors` is an array.

### Fix Focus Areas
- app/Views/public/ninja-footable-css.php[180-236]
- app/Views/public/ninja-footable-css.php[221-236]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

1 participant