Conversation
|
Thanks for the PR! 🎉 We've deployed an automatic preview for this PR - you can see your changes here:
Note The build needs to finish before your changes are deployed. |
| text = text.replace(/^[\t]*\n/, '').replace(/\n[\t]*$/, ''); | ||
| const templateMatch = text.match(/^[\t]*<template>[\n]*/); | ||
| this._isTemplate = templateMatch && templateMatch.length > 0; | ||
| text = text.replace(/^(\t*\n)*/, '').replace(/(\n\t*)*$/, ''); |
There was a problem hiding this comment.
The regex now handles multiple whitelines, before the code would break if 2 white lines where included in the demos. Not something we should expect but it should not break either
| // fix script whitespace | ||
| text = setIndent(text.replace(/\t/g, ' ')) | ||
| .replace(/<\/script>/g, '\n</script>') | ||
| .replace(/<script>/g, '<script>\n') | ||
| .replace(/<script type="module">/g, '<script type="module">\n') | ||
| .replace(/<script data-demo-hide(.+?)<\/script>/gis, '') | ||
| .split('\n'); | ||
| let scriptIndent = 0; | ||
| lines = lines.map((l) => { | ||
| if (l.indexOf('<script') > -1) { | ||
| scriptIndent = l.match(/^(\s*)/)[0].length; | ||
| return l; | ||
| } else if (l.indexOf('</script>') > -1) { | ||
| const nl = this._repeat(' ', scriptIndent) + l ; | ||
| scriptIndent = 0; | ||
| return nl; | ||
| } else if (scriptIndent && !this._isTemplate) { | ||
| return this._repeat(' ', scriptIndent + 2) + l; | ||
| } else { | ||
| return l; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Before we would look for script tags, and only fix indentation for those, now the same logic is applied to the whole snippet on setIndent. This is needed for when custom lit templates can be set, the function can handle cases where we want to ignore the first line (e.g. a template that starts inline but spans multiple lines html`<some-tag>\n ...) or we want to set an indentation level(e.g. nested templates)
| }); | ||
| } | ||
|
|
||
| _applyAttr(name, value, applyToShadowRoot) { |
There was a problem hiding this comment.
This was meant to handle setting rtl, that feature was removed so this is not needed anymore. This is now handled by _getDemoNodes() and _handleSkeletonChange()
| } | ||
| } | ||
|
|
||
| _repeat(value, times) { |
There was a problem hiding this comment.
This was a guard for string.repeat on IE11
|
I might ask, if there are any existing tests that will fail after these changes. or in other words, are there any tests that need to be updated ? if not, should new tests be added for these changes ? |
Good point, the tests are very barebones, I'll add some |
|
Hmm, looks like a test is timing out in Webkit... |
GZolla
dismissed
dlockhart’s stale review
Still need to figure out the timing issue
…spaceUI/core into gzolla/clean-demo-snippet
components/demo/demo-snippet.js
Outdated
| border: 1px solid var(--d2l-sem-border-color-standard); | ||
| border-radius: 6px; | ||
| box-shadow: 0 2px 2px 0 rgba(0, 0, 0, 0.14), 0 1px 5px 0 rgba(0, 0, 0, 0.12), 0 3px 1px -2px rgba(0, 0, 0, 0.2); | ||
| box-shadow: 0 2px 12px 0 rgba(0, 0, 0, 0.85); |
There was a problem hiding this comment.
I've replaced this box-shadow with the floating shadow from the semantic palette, it seems a lot more noticeable though
There was a problem hiding this comment.
Changed it to attached shadow, seems closer to the original
There was a problem hiding this comment.
The naming is totally up in the air at the moment. These attached variants are named with the top/bottom scroll cases in mind. We probably need something more general (going to talk with Jeff about it), but these are the values.
--d2l-shadow-attached-block-start: 0 2px 4px 0 rgba(0, 0, 0, 0.03);
--d2l-shadow-attached-block-end: 0 -2px 4px 0 rgba(0, 0, 0, 0.03);
--d2l-shadow-floating: 0 2px 12px 0 rgba(0, 0, 0, 0.15);
--d2l-shadow-inset: inset 0 2px 0 0 rgba(177, 185, 190, 0.2); /* corundum */4 participants
GAUD-9549
There is a lot of code on the demo snippet component that is no longer needed: