Skip to content

Commit 54e705c

Browse files
vursenvursen
committed
fix: address the review comments
1 parent 507361d commit 54e705c

File tree

6 files changed

+83
-68
lines changed

6 files changed

+83
-68
lines changed

‎packages/vaadin-grid/src/vaadin-grid-column.js‎

Lines changed: 66 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,12 @@ export const ColumnBaseMixin = (superClass) =>
133133
* Represents the final header renderer computed on the set of observable arguments.
134134
* It is supposed to be used internally when rendering the header cell content.
135135
*
136-
* @private
136+
* @protected
137137
* @type {GridHeaderFooterRenderer | undefined}
138138
*/
139-
__headerRenderer: {
139+
_headerRenderer: {
140140
type: Function,
141-
computed: '__computeHeaderRenderer(headerRenderer, _headerTemplate, header)'
141+
computed: '_computeHeaderRenderer(headerRenderer, _headerTemplate, header)'
142142
},
143143

144144
/**
@@ -156,12 +156,12 @@ export const ColumnBaseMixin = (superClass) =>
156156
* Represents the final footer renderer computed on the set of observable arguments.
157157
* It is supposed to be used internally when rendering the footer cell content.
158158
*
159-
* @private
159+
* @protected
160160
* @type {GridHeaderFooterRenderer | undefined}
161161
*/
162-
__footerRenderer: {
162+
_footerRenderer: {
163163
type: Function,
164-
computed: '__computeFooterRenderer(footerRenderer, _footerTemplate)'
164+
computed: '_computeFooterRenderer(footerRenderer, _footerTemplate)'
165165
}
166166
};
167167
}
@@ -174,9 +174,9 @@ export const ColumnBaseMixin = (superClass) =>
174174
'_textAlignChanged(textAlign, _cells.*, _headerCell, _footerCell)',
175175
'_orderChanged(_order, _headerCell, _footerCell, _cells.*)',
176176
'_lastFrozenChanged(_lastFrozen)',
177-
'__renderBodyCellsContent(_bodyTemplate, __renderer, _cells, _cells.*, path)',
178-
'__renderHeaderCellContent(_headerTemplate, __headerRenderer, _headerCell, path, header)',
179-
'__renderFooterCellContent(_footerTemplate, __footerRenderer, _footerCell)',
177+
'_onBodyTemplateOrRendererOrBindingChanged(_bodyTemplate, _renderer, _cells, _cells.*, path)',
178+
'_onHeaderTemplateOrRendererOrBindingChanged(_headerTemplate, _headerRenderer, _headerCell, path, header)',
179+
'_onFooterTemplateOrRendererOrBindingChanged(_footerTemplate, _footerRenderer, _footerCell)',
180180
'_resizableChanged(resizable, _headerCell)',
181181
'_reorderStatusChanged(_reorderStatus, _headerCell, _footerCell, _cells.*)',
182182
'_hiddenChanged(hidden, _headerCell, _footerCell, _cells.*)'
@@ -271,6 +271,14 @@ export const ColumnBaseMixin = (superClass) =>
271271
});
272272
}
273273

274+
/**
275+
* @protected
276+
*/
277+
_renderHeaderAndFooter() {
278+
this._renderHeaderCellContent(this._headerTemplate, this._headerRenderer, this._headerCell);
279+
this._renderFooterCellContent(this._footerTemplate, this._footerRenderer, this._footerCell);
280+
}
281+
274282
/**
275283
* @return {HTMLTemplateElement}
276284
* @protected
@@ -554,7 +562,7 @@ export const ColumnBaseMixin = (superClass) =>
554562

555563
cell._renderer = renderer;
556564

557-
if (model.item || renderer === this.__headerRenderer || renderer === this.__footerRenderer) {
565+
if (model.item || renderer === this._headerRenderer || renderer === this._footerRenderer) {
558566
this.__runRenderer(renderer, cell, model);
559567
}
560568
} else if (cell._template !== template) {
@@ -573,47 +581,63 @@ export const ColumnBaseMixin = (superClass) =>
573581
}
574582

575583
/**
576-
* Renders the content of body cells using either a template or a renderer.
584+
* Renders the header cell content using either a template or a renderer
585+
* and then updates the visibility of the parent row depending on
586+
* whether all its children cells are empty or not.
577587
*
578-
* @private
588+
* @protected
579589
*/
580-
__renderBodyCellsContent() {
581-
if (!this._cells) return;
582-
if (!this._bodyTemplate && !this.__renderer) return;
590+
_renderHeaderCellContent(headerTemplate, headerRenderer, headerCell) {
591+
if (!headerCell || (!headerTemplate && !headerRenderer)) {
592+
return;
593+
}
594+
595+
this.__renderCellsContent(headerTemplate, headerRenderer, [headerCell]);
596+
this._grid.__updateHeaderFooterRowVisibility(headerCell.parentElement);
597+
}
583598

584-
this.__renderCellsContent(this._bodyTemplate, this.__renderer, this._cells);
599+
/** @protected */
600+
_onHeaderTemplateOrRendererOrBindingChanged(headerTemplate, headerRenderer, headerCell, ..._bindings) {
601+
this._renderHeaderCellContent(headerTemplate, headerRenderer, headerCell);
585602
}
586603

587604
/**
588-
* Renders the header cell content using either a template or a renderer
589-
* and then updates the visibility of the parent row depending on
590-
* whether all its children cells are empty or not.
605+
* Renders the content of body cells using either a template or a renderer.
591606
*
592-
* @private
607+
* @protected
593608
*/
594-
__renderHeaderCellContent() {
595-
if (!this._headerCell) return;
596-
if (!this._headerTemplate && !this.__headerRenderer) return;
609+
_renderBodyCellsContent(template, renderer, cells) {
610+
if (!cells || (!template && !renderer)) {
611+
return;
612+
}
597613

598-
this.__renderCellsContent(this._headerTemplate, this.__headerRenderer, [this._headerCell]);
614+
this.__renderCellsContent(template, renderer, cells);
615+
}
599616

600-
this._grid.__updateHeaderFooterRowVisibility(this._headerCell.parentElement);
617+
/** @protected */
618+
_onBodyTemplateOrRendererOrBindingChanged(template, renderer, cells, ..._bindings) {
619+
this._renderBodyCellsContent(template, renderer, cells);
601620
}
602621

603622
/**
604623
* Renders the footer cell content using either a template or a renderer
605624
* and then updates the visibility of the parent row depending on
606625
* whether all its children cells are empty or not.
607626
*
608-
* @private
627+
* @protected
609628
*/
610-
__renderFooterCellContent() {
611-
if (!this._footerCell) return;
612-
if (!this._footerTemplate && !this.__footerRenderer) return;
629+
_renderFooterCellContent(footerTemplate, footerRenderer, footerCell) {
630+
if (!footerCell || (!footerTemplate && !footerRenderer)) {
631+
return;
632+
}
613633

614-
this.__renderCellsContent(this._footerTemplate, this.__footerRenderer, [this._footerCell]);
634+
this.__renderCellsContent(footerTemplate, footerRenderer, [footerCell]);
635+
this._grid.__updateHeaderFooterRowVisibility(footerCell.parentElement);
636+
}
615637

616-
this._grid.__updateHeaderFooterRowVisibility(this._footerCell.parentElement);
638+
/** @protected */
639+
_onFooterTemplateOrRendererOrBindingChanged(footerTemplate, footerRenderer, footerCell) {
640+
this._renderFooterCellContent(footerTemplate, footerRenderer, footerCell);
617641
}
618642

619643
/** @private */
@@ -662,14 +686,14 @@ export const ColumnBaseMixin = (superClass) =>
662686
__defaultFooterRenderer() {}
663687

664688
/**
665-
* Computes the final header renderer for the `__headerRenderer` computed property.
689+
* Computes the final header renderer for the `_headerRenderer` computed property.
666690
* All the arguments are observable by the Polymer, it re-calls the method
667691
* once an argument is changed to update the property value.
668692
*
669-
* @private
693+
* @protected
670694
* @return {GridHeaderFooterRenderer | undefined}
671695
*/
672-
__computeHeaderRenderer(headerRenderer, headerTemplate, header) {
696+
_computeHeaderRenderer(headerRenderer, headerTemplate, header) {
673697
if (headerRenderer) {
674698
return headerRenderer;
675699
}
@@ -684,11 +708,11 @@ export const ColumnBaseMixin = (superClass) =>
684708
}
685709

686710
/**
687-
* Computes the final renderer for the `__renderer` property.
711+
* Computes the final renderer for the `_renderer` property.
688712
* All the arguments are observable by the Polymer, it re-calls the method
689713
* once an argument is changed to update the property value.
690714
*
691-
* @private
715+
* @protected
692716
* @return {GridBodyRenderer | undefined}
693717
*/
694718
__computeRenderer(renderer, template) {
@@ -702,14 +726,14 @@ export const ColumnBaseMixin = (superClass) =>
702726
}
703727

704728
/**
705-
* Computes the final footer renderer for the `__footerRenderer` property.
729+
* Computes the final footer renderer for the `_footerRenderer` property.
706730
* All the arguments are observable by the Polymer, it re-calls the method
707731
* once an argument is changed to update the property value.
708732
*
709-
* @private
733+
* @protected
710734
* @return {GridHeaderFooterRenderer | undefined}
711735
*/
712-
__computeFooterRenderer(footerRenderer, footerTemplate) {
736+
_computeFooterRenderer(footerRenderer, footerTemplate) {
713737
if (footerRenderer) {
714738
return footerRenderer;
715739
}
@@ -777,12 +801,12 @@ class GridColumnElement extends ColumnBaseMixin(DirMixin(PolymerElement)) {
777801
* Represents the final renderer computed on the set of observable arguments.
778802
* It is supposed to be used internally when rendering the content of a body cell.
779803
*
780-
* @private
804+
* @protected
781805
* @type {GridBodyRenderer | undefined}
782806
*/
783-
__renderer: {
807+
_renderer: {
784808
type: Function,
785-
computed: '__computeRenderer(renderer, _bodyTemplate)'
809+
computed: '_computeRenderer(renderer, _bodyTemplate)'
786810
},
787811

788812
/**

‎packages/vaadin-grid/src/vaadin-grid-filter-column.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ class GridFilterColumnElement extends GridColumnElement {
3939
}
4040

4141
static get observers() {
42-
return ['__renderHeaderCellContent(_filterValue, path, header)'];
42+
return [
43+
'_onHeaderTemplateOrRendererOrBindingChanged(_headerTemplate, _headerRenderer, _headerCell, path, header, _filterValue)'
44+
];
4345
}
4446

4547
constructor() {
@@ -82,9 +84,9 @@ class GridFilterColumnElement extends GridColumnElement {
8284
* to override the header cell content.
8385
* It always renders the grid filter to the header cell.
8486
*
85-
* @private
87+
* @override
8688
*/
87-
__computeHeaderRenderer() {
89+
_computeHeaderRenderer() {
8890
return this.__defaultHeaderRenderer;
8991
}
9092

‎packages/vaadin-grid/src/vaadin-grid-selection-column.js‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ class GridSelectionColumnElement extends GridColumnElement {
9494
static get observers() {
9595
return [
9696
'__onSelectAllChanged(selectAll)',
97-
'__onDefaultHeaderRendererBindingChanged(__indeterminate, __selectAllHidden, selectAll)'
97+
'_onHeaderTemplateOrRendererOrBindingChanged(_headerTemplate, _headerRenderer, _headerCell, path, header, selectAll, __indeterminate, __selectAllHidden)'
9898
];
9999
}
100100

@@ -168,20 +168,6 @@ class GridSelectionColumnElement extends GridColumnElement {
168168
checkbox.checked = selected;
169169
}
170170

171-
/**
172-
* Re-renders the header cell content once a column property
173-
* bound in the default header renderer is changed.
174-
*
175-
* @private
176-
*/
177-
__onDefaultHeaderRendererBindingChanged() {
178-
if (this.__headerRenderer !== this.__defaultHeaderRenderer) {
179-
return;
180-
}
181-
182-
this.__renderHeaderCellContent();
183-
}
184-
185171
/** @private */
186172
__onSelectAllChanged(selectAll) {
187173
if (selectAll === undefined || !this._grid) {

‎packages/vaadin-grid/src/vaadin-grid-sort-column.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,9 @@ class GridSortColumnElement extends GridColumnElement {
4747
}
4848

4949
static get observers() {
50-
return ['__renderHeaderCellContent(direction, path, header)'];
50+
return [
51+
'_onHeaderTemplateOrRendererOrBindingChanged(_headerTemplate, _headerRenderer, _headerCell, path, header, direction)'
52+
];
5153
}
5254

5355
constructor() {
@@ -80,9 +82,9 @@ class GridSortColumnElement extends GridColumnElement {
8082
* to override the header cell content.
8183
* It always renders the grid sorter to the header cell.
8284
*
83-
* @private
85+
* @override
8486
*/
85-
__computeHeaderRenderer() {
87+
_computeHeaderRenderer() {
8688
return this.__defaultHeaderRenderer;
8789
}
8890

‎packages/vaadin-grid/src/vaadin-grid-tree-column.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ class GridTreeColumnElement extends GridColumnElement {
4343
}
4444

4545
static get observers() {
46-
return ['__renderBodyCellsContent(itemHasChildrenPath, path)'];
46+
return [
47+
'_onBodyTemplateOrRendererOrBindingChanged(_bodyTemplate, _renderer, _cells, _cells.*, path, itemHasChildrenPath)'
48+
];
4749
}
4850

4951
constructor() {
@@ -77,9 +79,9 @@ class GridTreeColumnElement extends GridColumnElement {
7779
* to override the content of body cells.
7880
* It always renders the grid tree toggle to body cells.
7981
*
80-
* @private
82+
* @override
8183
*/
82-
__computeRenderer() {
84+
_computeRenderer() {
8385
return this.__defaultRenderer;
8486
}
8587

‎packages/vaadin-grid/src/vaadin-grid.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -980,8 +980,7 @@ class GridElement extends ElementMixin(
980980
// header and footer renderers
981981
this._columnTree.forEach((level) => {
982982
level.forEach((column) => {
983-
column.__renderHeaderCellContent();
984-
column.__renderFooterCellContent();
983+
column.renderHeaderAndFooter();
985984
});
986985
});
987986

0 commit comments

Comments
 (0)