Skip to content
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

Use latest iron-component-page. #950

Merged
merged 12 commits into from
Aug 28, 2017
Merged

Use latest iron-component-page. #950

merged 12 commits into from
Aug 28, 2017

Conversation

manolo
Copy link
Member

@manolo manolo commented Aug 25, 2017

Connected to #878


This change is Reviewable

@tomivirkki
Copy link
Member

  • Wondering why <vaadin-grid-column-group> doesn't show up in the apidocs?
  • There's also a couple of behaviors listed (all behaviors can be removed)
  • <vaadin-grid-column> is showing connectedCallback and disconnectedCallback in the apidoc. Can those be marked private?
  • <vaadin-grid-filter> and -sorter are for some reason showing all these Polymer base api's like get and notifySplices. Any way to get them hidden?
  • <vaadin-grid-outer-scroller>, <vaadin-grid-scroller> and <vaadin-grid-templatizer> are not meant for public use? Is it possible to hide them from the apidoc without major hacks?

Review status: 0 of 28 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.


index.html, line 5 at r1 (raw file):

<html>
  <head>
    <title>vaadin-combo-box</title>

vaadin-grid


vaadin-grid-column-group.html, line 13 at r1 (raw file):

  <script>
    if (!Polymer.Element) {
      throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-column.html, line 345 at r1 (raw file):

  <script>
  if (!Polymer.Element) {
    throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-filter.html, line 28 at r1 (raw file):

  <script>
    if (!Polymer.Element) {
      throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-outer-scroller.html, line 34 at r1 (raw file):

<script>
  if (!Polymer.Element) {
    throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-scroller.html, line 13 at r1 (raw file):

<script>
  if (!Polymer.Element) {
    throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-selection-column.html, line 23 at r1 (raw file):

  <script>
  if (!Polymer.Element) {
    throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


vaadin-grid-sorter.html, line 87 at r1 (raw file):

  <script>
    if (!Polymer.Element) {
      throw new Error(`Unexpected Polymer version ${Polymer.version} is used, expected v2.0.0 or later.`);

One warning is enough (in vaadin-grid.html)


Comments from Reviewable

@manolo
Copy link
Member Author

manolo commented Aug 28, 2017

Wondering why doesn't show up in the apidocs?

It's because the concat in the observers block adding all super observers. I added a FIXME comment to fix it once we figure out a way to avoid that.

There's also a couple of behaviors listed (all behaviors can be removed)

I didn't see a way for excluding those behaviours by adding any annotation. Apparently the only way is not to pass them as arguments to the polymer cli, hence we need to maintain somehow a file with the list of the classes to analyse.

Trying to figure out why internal API is shown in certain cases.


Review status: 0 of 28 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.


Comments from Reviewable

@tomivirkki
Copy link
Member

I didn't see a way for excluding those behaviours by adding any annotation

I meant that they aren’t used by grid at all and the files can be deleted. Not a blocker for this PR


Comments from Reviewable

@manolo
Copy link
Member Author

manolo commented Aug 28, 2017


Review status: 0 of 30 files reviewed at latest revision, 8 unresolved discussions, some commit checks broke.


index.html, line 5 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

vaadin-grid

Done.


vaadin-grid-column-group.html, line 13 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-column.html, line 345 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-filter.html, line 28 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-outer-scroller.html, line 34 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-scroller.html, line 13 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-selection-column.html, line 23 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


vaadin-grid-sorter.html, line 87 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…

One warning is enough (in vaadin-grid.html)

Done.


Comments from Reviewable

@tomivirkki
Copy link
Member

:lgtm_strong:


Reviewed 17 of 28 files at r1, 13 of 13 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks broke.


Comments from Reviewable

@tomivirkki tomivirkki merged commit 22fcb9e into 4.0-preview Aug 28, 2017
@manolo manolo deleted the 4.0-fix/docs branch August 31, 2017 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants