Skip to content

Change how icons are rendered for inserting shortcodes to reduce numb…#2779

Merged
Crabcyborg merged 1 commit into
masterfrom
change_how_icons_are_rendered_to_reduce_number_of_phpcs_ignore_comments_for_echoing_unescaped_html
Jan 12, 2026
Merged

Change how icons are rendered for inserting shortcodes to reduce numb…#2779
Crabcyborg merged 1 commit into
masterfrom
change_how_icons_are_rendered_to_reduce_number_of_phpcs_ignore_comments_for_echoing_unescaped_html

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jan 12, 2026

Copy link
Copy Markdown
Contributor

…er of phpcs ignore comments for echoing unescaped html.

This update prevents the useless echo ''; as well.

…er of phpcs ignore comments for echoing unescaped html
@coderabbitai

coderabbitai Bot commented Jan 12, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The insert_opt_html method in FrmFormsHelper.php is refactored to change icon rendering. Instead of building an icon HTML string and escaping it with kses_icon(), the code now directly calls FrmAppHelper::icon_by_class() to output icons inline at two locations.

Changes

Cohort / File(s) Summary
Icon rendering refactoring in FrmFormsHelper
classes/helpers/FrmFormsHelper.php
Modified insert_opt_html method to replace FrmAppHelper::kses_icon() wrapping of manually-constructed icon HTML with direct calls to FrmAppHelper::icon_by_class() for inline icon output (2 occurrences)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • truongwp
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: refactoring how icons are rendered to reduce phpcs ignore comments for unescaped HTML echoing.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The PR description aligns with the changeset, which refactors icon rendering to reduce phpcs ignore comments for unescaped HTML output.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
classes/helpers/FrmFormsHelper.php (1)

708-721: Refactoring looks correct; consider simplifying the redundant checks.

The switch to FrmAppHelper::icon_by_class() is appropriate since it internally handles sanitization via kses_icon() when echoing, achieving the PR goal of reducing phpcs ignore comments.

However, since lines 688-689 ensure $field['icon'] is always set with a default value, the if ( isset( $field['icon'] ) ) checks on lines 708 and 718 are now always true and could be removed for cleaner code.

♻️ Optional: Remove redundant isset checks
 		?>
 		<li class="<?php echo esc_attr( $class ); ?>">
 			<a href="javascript:void(0)" class="frmids frm_insert_code" data-code="<?php echo esc_attr( $args['id'] ); ?>">
 				<?php
-				if ( isset( $field['icon'] ) ) {
-					FrmAppHelper::icon_by_class( $field['icon'], array( 'aria-hidden' => 'true' ) );
-				}
+				FrmAppHelper::icon_by_class( $field['icon'], array( 'aria-hidden' => 'true' ) );

 				echo esc_html( $truncated_name );
 				?>
 				<span>[<?php echo esc_attr( $args['id_label'] ?? $args['id'] ); ?>]</span>
 			</a>
 			<a href="javascript:void(0)" class="frmkeys frm_insert_code frm_hidden" data-code="<?php echo esc_attr( $args['key'] ); ?>">
 				<?php
-				if ( isset( $field['icon'] ) ) {
-					FrmAppHelper::icon_by_class( $field['icon'], array( 'aria-hidden' => 'true' ) );
-				}
+				FrmAppHelper::icon_by_class( $field['icon'], array( 'aria-hidden' => 'true' ) );

 				echo esc_html( $truncated_name );
 				?>
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6d46e43 and 043ac0e.

📒 Files selected for processing (1)
  • classes/helpers/FrmFormsHelper.php
🧰 Additional context used
🧬 Code graph analysis (1)
classes/helpers/FrmFormsHelper.php (1)
classes/helpers/FrmAppHelper.php (2)
  • FrmAppHelper (6-5027)
  • icon_by_class (1365-1406)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: Cypress
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 7.4 tests in WP 6.9

@Crabcyborg
Crabcyborg merged commit 2affd24 into master Jan 12, 2026
35 of 38 checks passed
@Crabcyborg
Crabcyborg deleted the change_how_icons_are_rendered_to_reduce_number_of_phpcs_ignore_comments_for_echoing_unescaped_html branch January 12, 2026 18:51
stephywells pushed a commit that referenced this pull request Apr 4, 2026
…d_to_reduce_number_of_phpcs_ignore_comments_for_echoing_unescaped_html

Change how icons are rendered for inserting shortcodes to reduce numb…
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.

1 participant