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

Polymer 2.x #814

Merged
merged 42 commits into from Nov 13, 2017

Conversation

Projects
None yet
2 participants
@bergie
Member

bergie commented Nov 10, 2017

We went to Polymer 1.x recently (#668). This pushes further, to a Polymer release generation that is properly available on NPM.

See:

Tasks:

  • Update to new WebComponents polyfill
  • Use the new <slot> API
  • Solve issue with Font Awesome icons (CSS doesn't apply inside shadow DOM)
  • Fix styling
@bergie

This comment has been minimized.

Show comment
Hide comment
@bergie

bergie Nov 10, 2017

Member

screenshot 2017-11-10 at 21 01 39

Styling generally looks fine now, the main remaining issue is how to load styles for CodeMirror and the-graph, since we can't load regular CSS files inside Shadow DOM.

Member

bergie commented Nov 10, 2017

screenshot 2017-11-10 at 21 01 39

Styling generally looks fine now, the main remaining issue is how to load styles for CodeMirror and the-graph, since we can't load regular CSS files inside Shadow DOM.

@bergie

This comment has been minimized.

Show comment
Hide comment
@bergie

bergie Nov 10, 2017

Member

As a bonus item, passing styling information to elements via CSS variables means now theming works better throughout

screenshot 2017-11-10 at 22 57 46

Member

bergie commented Nov 10, 2017

As a bonus item, passing styling information to elements via CSS variables means now theming works better throughout

screenshot 2017-11-10 at 22 57 46

@bergie bergie requested a review from jonnor Nov 10, 2017

@bergie

This comment has been minimized.

Show comment
Hide comment
@bergie

bergie Nov 10, 2017

Member

Also just did a quick testing round on iOS. Seems to work fine.

Member

bergie commented Nov 10, 2017

Also just did a quick testing round on iOS. Seems to work fine.

@bergie bergie referenced this pull request Nov 10, 2017

Closed

Better styles #280

@@ -272,18 +284,18 @@ <h1 on-keydown="checkUpdateName" on-blur="updateName" contenteditable="">{{graph
return;
}
this.htmlEditor = CodeMirror(this.$.html_editor, {
lineNumbers: true,
lineNumbers: false,

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

@bergie why lline numbers are disabled? This seems unrelated

@jonnor

jonnor Nov 13, 2017

Member

@bergie why lline numbers are disabled? This seems unrelated

This comment has been minimized.

@bergie

bergie Nov 13, 2017

Member

It seemed to trigger a rendering bug with CodeMirror when used in a narrow space

With line numbering:

screenshot 2017-11-13 at 21 11 30

Without:

screenshot 2017-11-13 at 21 11 41

@bergie

bergie Nov 13, 2017

Member

It seemed to trigger a rendering bug with CodeMirror when used in a narrow space

With line numbering:

screenshot 2017-11-13 at 21 11 30

Without:

screenshot 2017-11-13 at 21 11 41

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

Editing component code without line numbers is a bit shit...

@jonnor

jonnor Nov 13, 2017

Member

Editing component code without line numbers is a bit shit...

This comment has been minimized.

@bergie

bergie Nov 13, 2017

Member

This is not for component code, only for graph preview HTML (and graph specs)

@bergie

bergie Nov 13, 2017

Member

This is not for component code, only for graph preview HTML (and graph specs)

});
},
prepareTestsEditor: function () {
if (!this.spec) {
return;
}
this.testsEditor = CodeMirror(this.$.tests_editor, {
lineNumbers: true,
lineNumbers: false,

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

Same, why disabled?

@jonnor

jonnor Nov 13, 2017

Member

Same, why disabled?

This comment has been minimized.

@bergie
@bergie

bergie Nov 13, 2017

Member

See above

window.PolymerGestures.addEventListener(this, 'trackend', noop);
window.PolymerGestures.addEventListener(this, 'tap', noop);
}
// Pan graph by dragging map

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

@bergie is this feature removed or what?

@jonnor

jonnor Nov 13, 2017

Member

@bergie is this feature removed or what?

This comment has been minimized.

@bergie

bergie Nov 13, 2017

Member

It seems this never worked after we went Polymer 1.x

@bergie

bergie Nov 13, 2017

Member

It seems this never worked after we went Polymer 1.x

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

Might not have anything to do with Polymer 1.x, but with the-graph. There is a plain-React way to do it alo, see the-graph full example.

@jonnor

jonnor Nov 13, 2017

Member

Might not have anything to do with Polymer 1.x, but with the-graph. There is a plain-React way to do it alo, see the-graph full example.

}.bind(this));
// Tap to fit
this.addEventListener('tap', function () {
this.fire('triggerfit');

This comment has been minimized.

@jonnor

jonnor Nov 13, 2017

Member

And this feature?

@jonnor

jonnor Nov 13, 2017

Member

And this feature?

@jonnor

jonnor approved these changes Nov 13, 2017

YOLO

@bergie bergie merged commit 44accaa into master Nov 13, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@bergie bergie deleted the polymer_2x branch Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment