Conversation
wesalvaro
force-pushed
the
v2
branch
3 times, most recently
from
64dde07 to
e56eb2e
Compare
New Features: - More modular design - Less dependencies/complexity - Style sheets imported into Shadow DOM - Draws with (Chart)Wrapper - Introduces: - Dashboards - Controls - Queries - Editors - Call methods on charts - Chart Actions - Heavily updated demos Some known issues: - P0 Needs more tests! - P1 gViz bug? Drawing a WordTree shows an error - P3 gViz bug? Changing chart type has slow style sheet update due to delayed ready event.
| <template> | ||
| <style> | ||
| :host { | ||
| display: inline-block; |
There was a problem hiding this comment.
usually components that change display property of :host should also have:
:host([hidden]) {
display: none;
}| display: none; | ||
| } | ||
| </style> | ||
| <div id="control">Loading control...</div> |
There was a problem hiding this comment.
should users of the component be able to translate this string?
| /** | ||
| * The label of the column in the data to control. | ||
| * Either `label` or `index` should be set, not both. | ||
| * @type {string} |
There was a problem hiding this comment.
@type {?string}
string is non-nullable
| }, | ||
| /** | ||
| * The state of the specific control. | ||
| * @type {Object|undefined} |
There was a problem hiding this comment.
@type {!Object|undefined}
Object is nullable
| }, | ||
| /** | ||
| * Specifies the group for the chart in a Dashboard. | ||
| * @type {string} |
There was a problem hiding this comment.
@type {string|undefined}
property does not have a default value
| }, | ||
| _v: new GoogleChartLoader().visualization, | ||
| _onDataChanged(data) { | ||
| this.fire('google-chart-data-change', data); |
There was a problem hiding this comment.
this event does not have any documentation
can the event from notify: true property be used instead?
|
@rslawik Thanks for the review! |
|
@wesalvaro I'm guessing this would all have to be redone now that the code has been migrated to LitElement? |
|
We added few things from the list:
Not sure if we want to pursue the modular approach ( However, adding support for dashboards / controls / editors / action may still be worth exploring. Would those be useful to you? |
3 participants
New Features:
Some known issues: