Skip to content

Styling with additional Styles#28

Closed
WouterRademaker wants to merge 13 commits intoProfessionalWiki:masterfrom
WouterRademaker:styles
Closed

Styling with additional Styles#28
WouterRademaker wants to merge 13 commits intoProfessionalWiki:masterfrom
WouterRademaker:styles

Conversation

@WouterRademaker
Copy link
Contributor

@WouterRademaker WouterRademaker commented Feb 27, 2025

This can solve #6

Summary by CodeRabbit

  • Documentation

    • Updated usage instructions to explain how to apply custom styles when inserting FontAwesome icons, including an example for adding color and background options.
  • New Features

    • Enhanced icon rendering logic to support additional styling parameters when multiple arguments are provided, enabling more flexible icon customization.

@coderabbitai
Copy link

coderabbitai bot commented Feb 27, 2025

📝 Walkthrough

Walkthrough

The changes involve updates to the documentation and modifications to the icon rendering logic. The README.md file has been revised to clarify the usage of parser functions with additional styling parameters. In the JavascriptRenderer and WebfontRenderer classes, the render method has been altered to utilize a switch statement for handling varying numbers of arguments, allowing for the inclusion of a conditional style attribute when more than one argument is provided.

Changes

File(s) Change Summary
README.md Updated the usage section to describe parser functions ({{#far:...}}, {{#fas:...}}, {{#fab:...}}) including syntax for additional style parameters.
src/IconRenderers/JavascriptRenderer.php, src/IconRenderers/WebfontRenderer.php Modified the render method to use a switch statement that checks argument count; returns an element with a style attribute when more than one argument is given.

Possibly related PRs

  • Add documentation about styling with additional classes #25: The changes in the main PR enhance the documentation for using parser functions with additional styling parameters, while the retrieved PR adds details about using additional classes with the same parser functions, indicating a related focus on enhancing the usage documentation for FontAwesome icons.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d26e14 and 4510d67.

📒 Files selected for processing (1)
  • src/IconRenderers/WebfontRenderer.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/IconRenderers/WebfontRenderer.php

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

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 (7)
src/IconRenderers/JavascriptRenderer.php (2)

67-71: String used for integer comparison in switch statement

The switch statement uses the string literal "1" instead of an integer 1 when comparing the result of sizeof($args). While PHP may perform type juggling here, it's better to use the correct type for clarity and to avoid potential issues.

-		switch (sizeof($args)) {
-			case "1": return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
+		switch (sizeof($args)) {
+			case 1: return Html::element( 'i', [ 'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );

67-71: Improve code formatting and error handling

The current implementation has inconsistent indentation and doesn't handle the case when $args is empty.

-		switch (sizeof($args)) {
-			case "1": return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
-			default : return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ,
-						'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
-		}
+		$count = sizeof($args);
+		
+		if ($count < 1) {
+			return '';
+		}
+		
+		$attributes = [
+			'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ]
+		];
+		
+		if ($count > 1) {
+			$attributes['style'] = trim( $frame->expand( $args[ 1 ] ) );
+		}
+		
+		return Html::element( 'i', $attributes );
src/IconRenderers/WebfontRenderer.php (3)

65-69: String used for integer comparison in switch statement

The switch statement uses the string literal "1" instead of an integer 1 when comparing the result of sizeof($args). While PHP may perform type juggling here, it's better to use the correct type for clarity and to avoid potential issues.

-		switch (sizeof($args)) {
-			case "1": return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
+		switch (sizeof($args)) {
+			case 1: return Html::element( 'i', [ 'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );

65-69: Code duplication between WebfontRenderer and JavascriptRenderer

Both WebfontRenderer and JavascriptRenderer have nearly identical rendering logic, which creates maintenance challenges if changes are needed in the future.

Consider extracting the common rendering logic to a shared trait or base class to reduce duplication and make future maintenance easier.


65-69: Improve code formatting and error handling

The current implementation has inconsistent indentation and doesn't handle the case when $args is empty.

-		switch (sizeof($args)) {
-			case "1": return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
-			default : return Html::element( 'i', [ 	'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ,
-						'style' => trim( $frame->expand( $args[ 1 ] ) )                               ] );
-		}
+		$count = sizeof($args);
+		
+		if ($count < 1) {
+			return '';
+		}
+		
+		$attributes = [
+			'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ]
+		];
+		
+		if ($count > 1) {
+			$attributes['style'] = trim( $frame->expand( $args[ 1 ] ) );
+		}
+		
+		return Html::element( 'i', $attributes );
README.md (2)

72-74: Update unordered list style to use dashes

The static analysis tool flags that the list style uses asterisks instead of dashes, which is inconsistent with the expected Markdown style.

-* `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
-* `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
-* `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
+- `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
+- `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
+- `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

72-72: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


73-73: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


74-74: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


62-78: Consolidate parser function documentation

There is now duplication in the documentation, with two separate sections describing the parser functions. Consider consolidating these into a single, clear description.

-This extension defines three parser functions:
-* `{{#far:...}}` to insert an icon from the FontAwesome Regular font
-* `{{#fas:...}}` to insert an icon from the FontAwesome Solid font
-* `{{#fab:...}}` to insert an icon from the FontAwesome Brands font
-
-**Example:**
-`{{#fab:wikipedia-w}}` will insert the Wikipedia-W
-
-For valid icon names see https://fontawesome.com/v6/search.
-
-* `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
-* `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
-* `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
-
-**Example:**
-`{{#fab:wikipedia-w|color:red; background-color: green; }}`  will insert a red Wikipedia-W on a green background
+This extension defines three parser functions:
+- `{{#far:...}}` to insert an icon from the FontAwesome Regular font
+- `{{#fas:...}}` to insert an icon from the FontAwesome Solid font
+- `{{#fab:...}}` to insert an icon from the FontAwesome Brands font
+
+Each function can also accept an optional style parameter:
+- `{{#far:...|style}}` 
+- `{{#fas:...|style}}`
+- `{{#fab:...|style}}`
+
+**Examples:**
+- `{{#fab:wikipedia-w}}` will insert the Wikipedia-W
+- `{{#fab:wikipedia-w|color:red; background-color: green;}}` will insert a red Wikipedia-W on a green background
+
+For valid icon names see https://fontawesome.com/v6/search.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

63-63: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


64-64: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


65-65: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


70-70: Bare URL used
null

(MD034, no-bare-urls)


72-72: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


73-73: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


74-74: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f981435 and f027f3e.

📒 Files selected for processing (3)
  • README.md (1 hunks)
  • src/IconRenderers/JavascriptRenderer.php (2 hunks)
  • src/IconRenderers/WebfontRenderer.php (2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md

72-72: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


73-73: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


74-74: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

🔇 Additional comments (3)
src/IconRenderers/JavascriptRenderer.php (1)

69-70: Consider validating and sanitizing style attributes

The style attribute is directly inserted without validation. This could potentially be used for XSS attacks if the input is not properly sanitized elsewhere.

Consider adding validation or using MediaWiki's sanitization functions for the style attribute to ensure security.

src/IconRenderers/WebfontRenderer.php (1)

65-69: Consider validating and sanitizing style attributes

The style attribute is directly inserted without validation. This could potentially be used for XSS attacks if the input is not properly sanitized elsewhere.

Consider adding validation or using MediaWiki's sanitization functions for the style attribute to ensure security.

README.md (1)

76-78: Consider adding a security note about user-supplied styles

If these parser functions can be used by regular wiki users, it might be worth adding a note about potential security implications of using unsanitized styles, especially if user input is directly applied to HTML style attributes.

Consider adding a security note if these parser functions are exposed to untrusted users.

@WouterRademaker WouterRademaker changed the title Styling with a additional Styles Styling with additional Styles Feb 27, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

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)
README.md (1)

79-81: Unordered List Style Consistency

The new list items for the additional style parameters use asterisks (*), but markdownlint (MD004) expects dashes (-) for unordered list markers. Consider replacing the asterisks with dashes to maintain consistency with the project’s style guidelines and satisfy the static analysis tool.

-* `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
-* `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
-* `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
+ - `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
+ - `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
+ - `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

79-79: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


80-80: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


81-81: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f027f3e and 782d8d8.

📒 Files selected for processing (3)
  • README.md (1 hunks)
  • src/IconRenderers/JavascriptRenderer.php (1 hunks)
  • src/IconRenderers/WebfontRenderer.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/IconRenderers/WebfontRenderer.php
  • src/IconRenderers/JavascriptRenderer.php
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md

79-79: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


80-80: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


81-81: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

🔇 Additional comments (1)
README.md (1)

83-85: Clear Example for Additional Style Parameters

The new example showcasing the usage of additional style parameters is clear and instructive. It demonstrates how to apply inline CSS (e.g., color:red; background-color: green;) via the pipe (|) syntax. This added documentation helps users understand the enhanced functionality.

fix style errors
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (1)
src/IconRenderers/WebfontRenderer.php (1)

64-71: Consider edge case for empty args array.

The switch statement handles cases for one or more arguments, but doesn't explicitly handle the case where $args is empty. Consider adding a specific case for 0 to return a meaningful error or default icon.

	switch (count($args)) {
+		case 0:
+			return Html::element( 'span', ['class' => 'error'], 'FontAwesome: No icon specified' );
		case 1:
			return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
		default:
			return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
					'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
	}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 782d8d8 and 30a52ea.

📒 Files selected for processing (1)
  • src/IconRenderers/WebfontRenderer.php (4 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
src/IconRenderers/WebfontRenderer.php

[error] 41-43: Missing class property doc comment (MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPrivate)


[error] 62-62: Expected 0 blank lines after opening function brace; 1 found (Squiz.WhiteSpace.FunctionOpeningBraceSpace.SpacingAfter)


[error] 77-77: Expected 0 blank lines after opening function brace; 1 found (Squiz.WhiteSpace.FunctionOpeningBraceSpace.SpacingAfter)

🔇 Additional comments (3)
src/IconRenderers/WebfontRenderer.php (3)

2-2: Good addition of strict typing.

Adding declare(strict_types=1); improves code quality by enforcing strict type checking during function calls, which can prevent subtle bugs.


7-10: Documentation improvement.

Updated license and project description details are more accurate and complete.


92-92: Verify array construction in getFontModules.

The array construction syntax is a bit confusing. Is the first element intended to have the same key and value? Also, consider using consistent spacing.

-		return [ 'ext.fontawesome' => 'ext.fontawesome', 'ext.fontawesome.' . $this->fontClass ];
+		return [ 
+			'ext.fontawesome' => 'ext.fontawesome', 
+			'ext.fontawesome.' . $this->fontClass 
+		];

Please verify that this array structure is correct for ResourceLoader module registration.

Comment on lines 65 to 71
switch (count($args)) {
case "1":
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
default:
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider validating style input for security.

The code directly uses the user-provided style attribute without validation, which could potentially lead to XSS vulnerabilities.

Consider implementing style validation to ensure only safe CSS properties are allowed. For example:

private function validateStyle(string $style): string {
    // Simple validation example - implement more comprehensive validation
    $safeStyle = '';
    $properties = explode(';', $style);
    
    $allowedProperties = [
        'color', 'font-size', 'margin', 'padding',
        // Add more safe properties
    ];
    
    foreach ($properties as $property) {
        $parts = explode(':', $property, 2);
        if (count($parts) === 2) {
            $name = trim($parts[0]);
            if (in_array($name, $allowedProperties)) {
                $safeStyle .= $name . ':' . trim($parts[1]) . ';';
            }
        }
    }
    
    return $safeStyle;
}

Then apply this validation in the render method:

default:
    return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
-           'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
+           'style' => $this->validateStyle(trim( $frame->expand( $args[ 1 ] ) )) ] );

Comment on lines 41 to 43
private $fontClass;

private string $magicWord;
private $isModuleRegistered = false;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add missing PHPDoc for class properties.

The CI pipeline is reporting missing documentation for these private properties. Add proper PHPDoc comments to fix this issue.

+	/**
+	 * @var string CSS class for the font
+	 */
	private $fontClass;

+	/**
+	 * @var bool Tracks whether the ResourceLoader module has been registered
+	 */
	private $isModuleRegistered = false;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private $fontClass;
private string $magicWord;
private $isModuleRegistered = false;
/**
* @var string CSS class for the font
*/
private $fontClass;
/**
* @var bool Tracks whether the ResourceLoader module has been registered
*/
private $isModuleRegistered = false;
🧰 Tools
🪛 GitHub Actions: CI

[error] 41-43: Missing class property doc comment (MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPrivate)

Comment on lines 77 to 78
private function registerRlModule( Parser $parser ) {

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove extra blank line after function opening brace.

The CI pipeline is reporting an unexpected blank line after the opening brace.

	private function registerRlModule( Parser $parser ) {
-

		if ( $this->isModuleRegistered ) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private function registerRlModule( Parser $parser ) {
private function registerRlModule( Parser $parser ) {
if ( $this->isModuleRegistered ) {
🧰 Tools
🪛 GitHub Actions: CI

[error] 77-77: Expected 0 blank lines after opening function brace; 1 found (Squiz.WhiteSpace.FunctionOpeningBraceSpace.SpacingAfter)

Comment on lines 62 to 72
public function render( Parser $parser, PPFrame $frame, array $args ): string {
$this->registerRlModule( $parser );

return Html::element( 'i', [ 'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
$this->registerRlModule( $parser );
switch (count($args)) {
case "1":
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
default:
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove extra blank line after function opening brace.

The CI pipeline is reporting an unexpected blank line after the opening brace. Also, ensure string comparison uses the strict type check.

	public function render( Parser $parser, PPFrame $frame, array $args ): string {
-
		$this->registerRlModule( $parser );
		switch (count($args)) {
-			case "1":
+			case 1:
				return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
			default:
				return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
						'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
		}

This implementation correctly handles different numbers of arguments, allowing for styling customization when more than one argument is provided.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public function render( Parser $parser, PPFrame $frame, array $args ): string {
$this->registerRlModule( $parser );
return Html::element( 'i', [ 'class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
$this->registerRlModule( $parser );
switch (count($args)) {
case "1":
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
default:
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
}
}
public function render( Parser $parser, PPFrame $frame, array $args ): string {
$this->registerRlModule( $parser );
switch (count($args)) {
case 1:
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ] ] );
default:
return Html::element( 'i', ['class' => [ $this->fontClass, 'fa-' . trim( $frame->expand( $args[ 0 ] ) ) ],
'style' => trim( $frame->expand( $args[ 1 ] ) ) ] );
}
}
🧰 Tools
🪛 GitHub Actions: CI

[error] 62-62: Expected 0 blank lines after opening function brace; 1 found (Squiz.WhiteSpace.FunctionOpeningBraceSpace.SpacingAfter)

Copy link

@coderabbitai coderabbitai bot left a comment

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 (2)
README.md (2)

73-77: Enhance Consistency for Additional Classes Section

The updated "Styling" section now clearly demonstrates how to use additional classes with icons. However, static analysis (MD004) suggests that unordered list markers should use dashes (-) rather than asterisks (*) for consistency. Consider updating these list markers to adhere to the repository's markdown style guidelines.

-* `{{#far:... ...}}` to insert an icon from the FontAwesome Regular font with additional classes
-* `{{#fas:... ...}}` to insert an icon from the FontAwesome Solid font with additional classes
-* `{{#fab:... ...}}` to insert an icon from the FontAwesome Brands font with additional classes
+ - `{{#far:... ...}}` to insert an icon from the FontAwesome Regular font with additional classes
+ - `{{#fas:... ...}}` to insert an icon from the FontAwesome Solid font with additional classes
+ - `{{#fab:... ...}}` to insert an icon from the FontAwesome Brands font with additional classes
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

74-74: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


75-75: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


76-76: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


81-87: Enhance Consistency for Additional Style Section

The new examples for additional style parameters are well documented and illustrate the usage clearly. Similar to the previous section, the unordered list items currently use asterisks (*), whereas the markdown style guidelines (MD004) recommend dashes (-). Please update these to maintain consistency throughout the documentation.

-* `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
-* `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
-* `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
+ - `{{#far:...|...}}` to insert an icon from the FontAwesome Regular font with additional style
+ - `{{#fas:...|...}}` to insert an icon from the FontAwesome Solid font with additional style
+ - `{{#fab:...|...}}` to insert an icon from the FontAwesome Brands font with additional style
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

81-81: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


82-82: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


83-83: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cfa208c and a9896cf.

📒 Files selected for processing (2)
  • README.md (1 hunks)
  • src/IconRenderers/JavascriptRenderer.php (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/IconRenderers/JavascriptRenderer.php
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md

74-74: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


75-75: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


76-76: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


81-81: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


82-82: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)


83-83: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

@WouterRademaker
Copy link
Contributor Author

Replaced by #38

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