-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added bundle and thumbnail in RFQ #1269
Conversation
Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
if (data.bundleThumbnail) { | ||
if (!data.bundleThumbnail.startsWith('https')) { | ||
if (data.bundleThumbnail.startsWith('.')) { | ||
data.bundleThumbnail = data.bundleThumbnail.substring(1); | ||
} | ||
data.bundleThumbnail = `https://www.moleculardevices.com${data.bundleThumbnail}`; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this logic is similar to the product image we should move that to a function instead of duplication.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function hasThumbnailImage created for the same
if (data.path) { | ||
hubSpotQuery.website = `https://www.moleculardevices.com${data.path}`; | ||
} else { | ||
hubSpotQuery.website = `https://www.moleculardevices.com${data.toLowerCase()}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? Data is an object white multiple fields. I think we can set the website to the hose only allready when hubSpotQuery
is created above and only if data.path exists we append it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, else part removed!
|
|
|
|
@@ -130,9 +130,20 @@ function iframeResizehandler(formUrl, id, root) { | |||
}); | |||
} | |||
|
|||
function hasThumbnailImage(thumbImage) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe rename to prepImageUrl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done @mhaack
|
|
|
|
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #
Test URLs: