fix widget link issues #91
Conversation
| @@ -62,6 +64,14 @@ const GridTile = ({ widgetType, isDragging, setIsDragging, setWidgetAttribute, w | |||
| return null; | |||
| } | |||
|
|
|||
| const widgetLink = (href: string) => { | |||
| if (href.includes('https://')) { | |||
There was a problem hiding this comment.
is there a better way to check if the href is an external link?
There was a problem hiding this comment.
This is not reliable. We use a isExternal prop in our links to determine if a link is external. Configuration is responsible for marking external links.
There was a problem hiding this comment.
I'm not seeing an isExternal prop being passed down
There was a problem hiding this comment.
I don't think exists yet in the widgets. But its a pattern all across the platform.
| @@ -192,7 +202,8 @@ const GridTile = ({ widgetType, isDragging, setIsDragging, setWidgetAttribute, w | |||
| <Button | |||
There was a problem hiding this comment.
Please use react-router-dom Links when we are not opening new tabs and staying in HCC.
|
I think the styling is now off.
|
|
@Hyperkid123 @InsaneZein Styling looks fine now |
| if (href.includes('https://')) { | ||
| return href; | ||
| } else { | ||
| return `${window.location.origin}${chrome.isBeta() ? '/preview' : ''}${href}`; |
There was a problem hiding this comment.
You don't have to add origin or the preview partial to router links. That is handled via the component.
| {headerLink.title} | ||
| </Button> | ||
| {headerLink.href && ( | ||
| <Link to={widgetLink(headerLink.href)}> |
There was a problem hiding this comment.
You can't mix client-side routing with classic HTML routing. You have to pick a link component based on what the target is.
// for origin console.redhat.com
const href = https://www.google.com/
<Link to={href}>...</Link>
// actual link: console.redhat.com/https://www.google.com/You have to decide which type of a link you are supposed to use.
There was a problem hiding this comment.
I'm not sure I understand. Do I pick a different routing component based on whether its external or not?
There was a problem hiding this comment.
Exactly. For external links, you use the plain HTML tags.
| @@ -62,6 +64,14 @@ const GridTile = ({ widgetType, isDragging, setIsDragging, setWidgetAttribute, w | |||
| return null; | |||
| } | |||
|
|
|||
| const widgetLink = (href: string) => { | |||
| if (href.includes('https://')) { | |||
There was a problem hiding this comment.
This is not reliable. We use a isExternal prop in our links to determine if a link is external. Configuration is responsible for marking external links.
Hyperkid123
dismissed
epwinchell’s stale review
PR has still some code issues to be addressed.
@Hyperkid123 I'm not seeing an isExternal link prop passed anywhere. |
Description
Fixes RHCLOUD-32859
Screenshots
After:
Checklist ☑️