Remove external link icons#43937
Conversation
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…assistant.io into remove-iconify-icon
There was a problem hiding this comment.
Pull request overview
Removes the external-link icon injection/styling to avoid formatting issues in Home Assistant documentation.
Changes:
- Removed SCSS rules that positioned/added spacing for external link icons.
- Removed Nokogiri logic that appended
iconify-iconnodes to external links. - Left link-handling logic in place to skip certain “own site” URLs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sass/homeassistant/_overrides.scss | Removes styling/layout rules for appended external-link icons. |
| plugins/output_modder.rb | Stops injecting external-link icons into external anchors. |
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| # Append an external link icon, if there isn't an icon already | ||
| # Skip our own links | ||
| next if link.get_attribute('href') =~ /\Ahttps?:\/\/\w*.?home-assistant.io/i |
There was a problem hiding this comment.
The regex on line 25 is overly permissive: .? uses an unescaped . (matches any character), and home-assistant.io is not escaped (the . matches any character). This can cause unintended matches/skips. Use a stricter pattern (escape dots, and use an explicit optional subdomain group), or reuse the safer domain allowlist regex already present below.
| next if link.get_attribute('href') =~ /\Ahttps?:\/\/\w*.?home-assistant.io/i | |
| next if link.get_attribute('href') =~ /\Ahttps?:\/\/(?:\w+\.)?home-assistant\.io/i |
There was a problem hiding this comment.
@copilot can you give me some examples where this might be falsely matched?
|
@mrdarrengriffin I've opened a new pull request, #43938, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
Copilot went AWOL. Opening new, cleaner PR |
Proposed change
The external link icons cause too many formatting issues to warrant keeping in. This language is only used on Home Assistant documentation and are not present in any of our other websites.
To improve readability, I propose removing these
Type of change
currentbranch).currentbranch).nextbranch).nextbranch).Additional information
Checklist
currentbranch.nextbranch.