Extend icon plugin with title attribute #36004
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
I'm not sure where you demo normally that these things work, I just added them to lamarzocco for verification, but that needs to be removed. |
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Jekyll
participant Icon
User->>Jekyll: Request icon with title
Jekyll->>Icon: Initialize with title
Icon-->>Jekyll: Render icon with title
Jekyll-->>User: Display icon with title
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
plugins/icon.rb (1)
29-29: LGTM! Consider adding a comment explaining the regex patternThe regex pattern is well-structured and correctly handles the optional title attribute. Consider adding a comment explaining the pattern components for better maintainability.
Add a comment like this:
+ # Matches icon tags in format: "namespace:icon-name" [title="Optional title"] + # namespace and icon-name must be in kebab-case (lowercase with optional hyphens) SYNTAX = %r!^"(?<icon>[a-z0-9]+(?:-[a-z0-9]+)*:[a-z0-9]+(?:-[a-z0-9]+)*)"(?:\s+title="(?<title>[^"]+)")?$!.freeze
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
plugins/icon.rb(1 hunks)source/_integrations/lamarzocco.markdown(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
plugins/icon.rb (6)
Pattern */**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md): do not comment on HTML used for icons
Pattern */**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (1)
plugins/icon.rb (1)
17-17: LGTM!
The error message clearly shows the updated syntax including the optional title attribute.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
plugins/icon.rb (1)
22-30: Enhance title formatting for better readabilityThe current implementation only capitalizes the first word. Consider improving the formatting for better readability.
def default_title(icon) # split away "mdi:" part icon_name = icon.split(':').last # remove dashes and capitalize e.g. "water-polo" to "Water polo" - parts = icon_name.split('-') - parts[0] = parts[0].capitalize - parts.join(' ') + # Capitalize each word for better readability + icon_name.split('-').map(&:capitalize).join(' ') end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
plugins/icon.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
plugins/icon.rb (6)
Pattern */**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md): do not comment on HTML used for icons
Pattern */**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (4)
plugins/icon.rb (4)
11-17: LGTM!
The error message is clear and provides helpful guidance for the correct syntax.
39-39: LGTM!
The regex pattern effectively handles the optional title parameter and uses named capture groups appropriately.
8-9:
Sanitize the title attribute value to prevent XSS
The title value should be sanitized to prevent potential XSS attacks when rendered as HTML attributes.
- @title = Regexp.last_match[:title]
+ @title = CGI.escape_html(Regexp.last_match[:title]) if Regexp.last_match[:title]Don't forget to add the required import at the top of the file:
require 'cgi'33-34:
Use double quotes for HTML attributes and escape properly
The current implementation could break if the title contains quotes.
- title = @title || default_title(@icon)
- "<iconify-icon inline icon='#{@icon}' title='#{title}'></iconify-icon>"
+ title = @title || default_title(@icon)
+ %(<iconify-icon inline icon="#{@icon}" title="#{title.gsub('"', '"')}"></iconify-icon>)
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Proposed change
SSIA
Type of change
currentbranch).currentbranch).nextbranch).nextbranch).Additional information
Checklist
currentbranch.nextbranch.Summary by CodeRabbit
New Features
iconLiquid tag to support an optional title for improved usability in Jekyll templates.Documentation