-
Notifications
You must be signed in to change notification settings - Fork 344
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
Infrastructure: Codepen button should show only when resources loaded #1533
Conversation
examples/js/examples.js
Outdated
@@ -378,32 +380,28 @@ function addOpenInCodePenForm (exampleIndex, exampleHeaderId, exampleCodeId, exa | |||
totalFetchedFiles++; | |||
} | |||
else { | |||
hideButton(buttonId, "Could not load resource: " + href); | |||
console.log("Not showing 'Open in Codepen' button. Could not load resource: " + href); |
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 it make sense to warn/error for these?
console.log("Not showing 'Open in Codepen' button. Could not load resource: " + href); | |
console.warn("Not showing 'Open in Codepen' button. Could not load resource: " + href); |
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.
makes sense! pushed commit for switch.
@smhigley friendly ping when you get time |
Regression test coverage:Examples without any regression tests:
Examples missing some regression tests:
Example pages with Keyboard or Attribute table rows that do not have data-test-ids:
SUMMARY:55 example pages found. ERROR - missing tests: Please write missing tests for this report to pass. |
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.
love 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.
+1
Looks good!
Thank you @smhigley and @carmacleod ... will merge! |
It was a good point @smhigley to show the button after the resources have been loaded instead of hiding it if they don't! :)
I didn't implement the promise array because XMLHttpRequest.send doesn't return a promise but that it's true that would have been much cleaner.