search

vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
429 stars 82 forks source link

[grid] Allow custom prefix for GridElement-Extension #1449

Open probert94 opened 5 years ago

probert94 commented 5 years ago

Description

I would like to extend the vaadin-grid with custom behaviour. I used the implementation of the vaadin-grid-pro as a reference and created a custom-grid without additional logic.

Expected outcome

The custom-grid should be rendered as the vaadin-grid.

Actual outcome

The following error is thrown:

Uncaught TypeError: Cannot read property '__updateHeaderFooterRowVisibility' of undefined at HTMLElement._pathOrHeaderChanged (vaadin-grid-column.html:377) at Object.runMethodEffect [as fn] (property-effects.html:818) at runEffectsForProperty (property-effects.html:162) at runEffects (property-effects.html:128) at HTMLElement._propertiesChanged (property-effects.html:1712) at HTMLElement.s._propertiesChanged (client-28831013082792340517B7074033EF04.cache.js:983) at HTMLElement._flushProperties (properties-changed.html:341) at HTMLElement._flushProperties (property-effects.html:1560) at HTMLElement._invalidateProperties (property-effects.html:1532) at HTMLElement._setProperty (property-effects.html:1517) at HTMLElement.Object.defineProperty.set (properties-changed.html:150) at columns.forEach (vaadin-grid.html:492) at Array.forEach () at HTMLElement._updateRow (vaadin-grid.html:487) at HTMLElement._createScrollerRows (vaadin-grid.html:410) at HTMLElement._createPool (vaadin-grid-scroller.html:219) at HTMLElement._increasePoolIfNeeded (iron-list.html:497) at HTMLElement._increasePoolIfNeeded (vaadin-grid-scroller.html:178) at dataProvider (vaadin-grid-data-provider-mixin.html:327) at Object.grid.$connector.confirm (gridConnector.js:813) at Object.eval (eval at jt (client-28831013082792340517B7074033EF04.cache.js:975), :3:15) at jt (client-28831013082792340517B7074033EF04.cache.js:975) at it (client-28831013082792340517B7074033EF04.cache.js:931) at gt (client-28831013082792340517B7074033EF04.cache.js:562) at Pq (client-28831013082792340517B7074033EF04.cache.js:482) at lr.mr [as W] (client-28831013082792340517B7074033EF04.cache.js:983) at eA (client-28831013082792340517B7074033EF04.cache.js:866) at Rq (client-28831013082792340517B7074033EF04.cache.js:978) at fr.gr [as D] (client-28831013082792340517B7074033EF04.cache.js:983) at Bj (client-28831013082792340517B7074033EF04.cache.js:409) at Jq (client-28831013082792340517B7074033EF04.cache.js:979) at Kq (client-28831013082792340517B7074033EF04.cache.js:963) at Ws.Ys [as qb] (client-28831013082792340517B7074033EF04.cache.js:983) at IA.JA [as K] (client-28831013082792340517B7074033EF04.cache.js:983) at XMLHttpRequest. (client-28831013082792340517B7074033EF04.cache.js:572) at sb (client-28831013082792340517B7074033EF04.cache.js:413) at vb (client-28831013082792340517B7074033EF04.cache.js:850) at XMLHttpRequest. (client-28831013082792340517B7074033EF04.cache.js:592)

Notes

The reason for the error is, that in vaadin-grid-column#_findHostGrid, the localName of the GridElement is expected to match /^vaadin.*grid(-pro)?$/. When I rename the custom-grid to vaadin-custom-grid, it works as expected.

I understand, that you have to search for the GridElement using some restricted name, but in my opinion the current logic is too restrictive, as it enforces a vaadin-prefix, which is usually used by official components only.

web-padawan commented 5 years ago

You are right, this line is causing it: https://github.com/vaadin/vaadin-grid/blob/77defe3be558b3ba10ce21e7a6c87b38403a5790/src/vaadin-grid-column.html#L178-L179

This has been implemented in vaadin/vaadin-grid#1532 and previously submitted as vaadin/vaadin-grid#1416

We have this kind of "naming convention" because of the fact that we can't check el instanceof GridElement in Polymer 3 version, as the order of upgrading custom elements is not guaranteed and apparently in some cases this code is called before vaadin-grid has upgraded.

probert94 commented 5 years ago

@web-padawan thank you for sharing the information. In your opinion, would it be possible to allow a custom (non vaadin) prefix? Following regex would, for example, allow a single custom prefix, but the component has to end with grid or grid-pro: ^[a-z A-Z 0-9]+(-grid(-pro)?$)

tomivirkki commented 5 years ago

👍 I don't see a reason why the column should only accept vaadin- prefixed hosts.

Workaround: If you're extending the grid with a custom prefix, you could also extend the column class and override the protected _findHostGrid in the extension:

<my-grid>
  <my-column></my-column>
  <my-column></my-column>
</my-grid>
probert94 commented 5 years ago

@tomivirkki thank you for sharing the workaround. As I am using Vaadin Flow, the workaround will probably be a bit more complex, so I will use the vaadin-prefix for now. But I'm glad you share my opinion about the custom prefix.