fix: fixed for auro-hyperlink not being rendered when relative and ta…#312
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances ComponentBase to correctly normalize and detect relative URLs as internal links, and updates AuroHyperlink to allow rendering the target attribute for those relative links. Class diagram for updated ComponentBase and AuroHyperlinkclassDiagram
class ComponentBase {
+isRelativeUrl(href)
+normalizeUrl(href)
+isAlaskaAirDomain(url)
+relative : boolean
}
class AuroHyperlink {
+target : string
+includesDomain : boolean
+relative : boolean
}
ComponentBase <|-- AuroHyperlink
Flow diagram for target attribute rendering logic in AuroHyperlinkflowchart TD
A["User provides href to AuroHyperlink"] --> B["Check if href is relative"]
B -->|Yes| C["Set relative = true"]
B -->|No| D["Check if href includes AlaskaAir domain"]
C --> E["target attribute rendered if target is set"]
D --> F["target attribute rendered if target is set and includesDomain is true"]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider adding a guard in isRelativeUrl to handle non-string or empty inputs so you don’t run into runtime errors when url is undefined or null.
- You might extract isRelativeUrl and the domain check into a shared URL utility module for better reuse across components and easier standalone testing.
- The regex in isRelativeUrl currently only blocks http(s) and protocol-relative URLs; consider extending it to explicitly exclude other schemes (e.g., mailto:, javascript:) if those should not be treated as relative.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a guard in isRelativeUrl to handle non-string or empty inputs so you don’t run into runtime errors when url is undefined or null.
- You might extract isRelativeUrl and the domain check into a shared URL utility module for better reuse across components and easier standalone testing.
- The regex in isRelativeUrl currently only blocks http(s) and protocol-relative URLs; consider extending it to explicitly exclude other schemes (e.g., mailto:, javascript:) if those should not be treated as relative.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Something here is still broken. To reproduce this issue, I used this code:
<auro-hyperlink target="_blank" href="/assets/icons/plane.svg" relative>No href supplied</auro-hyperlink><br>
Welcome to <auro-hyperlink target="_blank" href="/assets/icons/plane.svg" relative>Alaska Airlines</auro-hyperlink>.
When testing, I noticed that the first link rendered just fine, but the second link with text surrounding the component is still broke. Let me know if you do not see the same thing and need to discuss this further.
The changes requested are done
315e20e to
1030bc3
Compare
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/component-base.mjs:258-262` </location>
<code_context>
return url.href;
}
- return href.replace(/^[^:]+:/, "");
+ /**
+ * This regex checks if the provided href starts with http:, http:, ftp:, etc...
+ * Also checks if it has // if that matches it replaces it for just one /
+ */
+ return href.replace(/^(?:[^:]+:)?\/\/?/, "/");
}
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The updated regex may unintentionally convert protocol-relative URLs to root-relative paths.
This change may break links intended to preserve the host. Please confirm if this behavior is intentional and appropriate for all use cases.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
🚀 PR Release Published! To install: npm install @aurodesignsystem-dev/auro-hyperlink@0.0.0-pr312.1Install via alias: npm install @aurodesignsystem/auro-hyperlink@npm:@aurodesignsystem-dev/auro-hyperlink@0.0.0-pr312.1 |
1030bc3 to
e8310d2
Compare
|
🎉 This PR is included in version 7.0.0-rc-320.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 8.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 7.0.0-rc-329.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 7.0.0-rc-336.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 7.0.0-rc-339.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…rget='_blank' are set
Alaska Airlines Pull Request
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Checklist:
By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Fix auro-hyperlink support for relative URLs by normalizing hrefs properly and treating them as internal so target attributes are applied
Bug Fixes: