- Notifications
You must be signed in to change notification settings - Fork 2.4k
Stackblitz demos for 3.x elements #2537
Conversation
keanulee left a comment
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.
Do you have a sample deployment? Maybe @arthurevans or @katejeffreys should try the gulp/deploy process?
app/elements/demo-tabs.html Outdated
| | ||
| // Call out to a global method defined by app.js | ||
| _plunkerLaunched: function() { | ||
| _hideEditorButton: function(srcDefined, prjoectPathDefined) { |
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.
_hideEditorButton is a method AND a property?
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.
oops
| | ||
| _isStackBlitz: { | ||
| type: Boolean, | ||
| }, |
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.
_isPlunkr and _isStackBlitz unused
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.
ah I forgot to rename srcDefined to this etc.
app/elements/demo-tabs.html Outdated
| return (!src || src === ''); | ||
| _srcProjectPathChanged: function(src, projectPath) { | ||
| this._srcDefined = !!this.src; | ||
| this._projectPathDefined = !!this.projectPath; |
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.
Feel like _srcDefined and _projectPathDefined aren't needed since they're just proxies for src/projectPath
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.
In the process of removing them, but I'm going to need one that is of type boolean as the databinding will not work if I try
<div hidden$=[[!stringThatIsUndefined]]>
app/elements/stack-blitz.html Outdated
| @@ -0,0 +1,186 @@ | |||
| <link rel="import" href="./stack-blitz-sdk.html"> | |||
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.
stack-blitz-sdk.html doesn't need to exist (not used elsewhere so doesn't need to be de-duped) - just have <script src="../js/sdk.umd.js"></script> here
| }.bind(this)).catch(onError); | ||
| }.bind(this); | ||
| | ||
| this.importHref('/elements/stack-blitz.html', onLoad, onError, true); |
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.
💯 👍
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.
changes implemented
| Hey everyone- StackBlitz co-creator here! Super excited to see this PR :) Just wanted to give y'all an update on the ETA for the various items outlined above from our end. With ngconf done and our Teleport feature out the door, we're finally able to focus on the next version of our bundler/dev server which solves all the 'critical' & 'nice to have' items. Specifically, we're going to enable serving the static JS files themselves without any transpilation done beforehand. We're realistically shooting to have this live on prod in ~4 weeks- would love to hear everyone's thoughts/if there's func in polymer CLI that'd be ideal to have on StackBlitz as well 🥂 |
| @EricSimons TBH it's miraculous that bare module specifiers work out of the box for ES6 modules with no additional tooling. There is nothing really polymer-cli-specific that I can think of atm, but there are a few rough patches that we've been filing issues for. Perhaps we can sync up later sometime in VC to discuss this further? |
| Sounds good! My email is eric at esft.com, so feel free to drop me a note and we'll get something on the calendar 👍 |
| \o/ i will try to review this Thursday 26th |
package.json Outdated
| "scripts": { | ||
| "postinstall": "bower install; gulp", | ||
| "postinstall": "bower install; npm run generate-stackblitz; gulp", | ||
| "generate-stackblitz": "cp ./node_modules/@stackblitz/sdk/bundles/sdk.umd.js ./app/js/sdk.umd.js; cp ./node_modules/@stackblitz/sdk/bundles/sdk.umd.js.map ./app/js/sdk.umd.js.map;", |
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.
Can we use a hosted version of the SDK instead? Not a huge fan of this one-off, manual copying from node_modules/.
<script src="https://unpkg.com/@stackblitz/sdk/bundles/sdk.umd.js"></script>(referenced https://medium.com/@ericsimons/stackblitz-sdk-instantly-add-live-environments-to-your-docs-blogs-more-73dab05c51ae)
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.
Sure, I'll just need to add this to the service worker
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.
spoke offline, decided it's best not to add SW
.gitignore Outdated
| # Dev directories | ||
| /app/bower_components | ||
| /node_modules | ||
| /app/node_modules |
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.
Don't think this is created.
package.json Outdated
| }, | ||
| "homepage": "https://www.polymer-project.org/1.0/", | ||
| "devDependencies": { | ||
| "@stackblitz/sdk": "^1.1.1", |
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.
Reset this file and package-lock.json
.gitignore Outdated
| # Generated files/folders | ||
| *.pyc | ||
| app/css/* | ||
| app/js/sdk* |
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.
Remove this
app/elements/stack-blitz.html Outdated
| @@ -0,0 +1,187 @@ | |||
| <link rel="import" href="../bower_components/polymer/polymer.html"> | |||
| | |||
| <script src="https://unpkg.com/@stackblitz/sdk/bundles/sdk.umd.js"></script> | |||
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.
Pin unpkg to a specific version to avoid breakages/extra redirect https://unpkg.com/@stackblitz/sdk@1.1.1/bundles/sdk.umd.js
gulpfile.js Outdated
| return gulp.src([ | ||
| 'gruntfile.js', | ||
| 'app/js/**/*.js', | ||
| '!app/js/**/sdk*', |
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.
argh, one more thing to remove
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.
I'm just testing you... you pass
keanulee left a comment
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.
Code changes LGTM. Will defer to @arthurevans and/or @katejeffreys to test on their machines as well.
ghost left a comment
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.
Tested, works beautifully on my machine. Thank you so much for this!!
The commit messages might be helpful to follow for review here.
What I've done:
I added inline demos for
3.0. It uses stackblitz. See gif at the bottom of this PR for exampleadded thegenerate-stackblitznpm command which copies it from node modules to a servable folder in the postinstallstack-blitzelement<stack-blitz projectPath="/path/to/project"></stack-blitz>demo-tabselement to implement stack-blitzstack-blitzis lazy loaded1.0and `2.0 examples3.0docs to use this new stackblitz demo tabs3.0samples to use the manifestUsage:
index.htmland anindex.jsmanifest.jsonat the root of that folder{ files: string[], dependencies: { [npmDepName: string]: string }}srcwhich was used for plunkr links, useprojectPathwhich is the path to the sample folder you just created (e.g./3.0/samples/custom-element)editorOpenFileto declare what file will be shown upon editor embedPerformance
3.0 quick-tour Chrome with service worker and cache on
first time page load
subsequent page load
Lighthouse
Lighthouse score went up to high 60s from low 60s as we removed the iframe demos and lazy load the editor.
Outstanding issues
Critical
Importing an element that usespolymer-legacy.jscauses compile error Cannot read property 'end' of undefined stackblitz/core#441Nice to have
writenode_modulesinstead ofturbo_modulesInstall dependencies into node_modules, not turbo_modules stackblitz/core#333Demo
Try it out! Link
