vaadin / expense-manager-demo

Progressive Web App (PWA) demo using Vaadin components
https://vaadin.com/components
Apache License 2.0
360 stars 142 forks source link

Fix console errors since content has a shadow-root #88

Closed manolo closed 6 years ago

manolo commented 6 years ago

This change is Reviewable

tomivirkki commented 6 years ago

src/expense-editor.html, line 188 at r1 (raw file):

         */
        _getDialogChild(selector) {
          return this.$.dialog.$.overlay.$.content.root.querySelector(selector);

Should be "shadowRoot" instead of "root"


Comments from Reviewable

manolo commented 6 years ago

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.


src/expense-editor.html, line 188 at r1 (raw file):

Previously, tomivirkki (Tomi Virkki) wrote…
Should be "shadowRoot" instead of "root"

Done.


Comments from Reviewable

web-padawan commented 6 years ago

Reviewed 1 of 2 files at r1, 2 of 2 files at r2. Review status: all files reviewed at latest revision, 1 unresolved discussion.


src/expense-editor.html, line 77 at r2 (raw file):

            <div class="file-list">
              <div class="receipt-wrapper">
                <img src$="[[rootPath]][[_expense.receipt]]" alt="Receipt" hidden$="[[!_expense.receipt]]">

why has the ` been removed


Comments from Reviewable

web-padawan commented 6 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions.


src/expense-editor.html, line 77 at r2 (raw file):

Previously, web-padawan (Sergey Kulikov) wrote…
why has the ` been removed

sorry, I mean "rootPath". It is needed to work with prpl-server, and adds the "/es6-bundled/" folder prefix here


Comments from Reviewable

manolo commented 6 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions.


src/expense-editor.html, line 77 at r2 (raw file):

Previously, web-padawan (Sergey Kulikov) wrote…
sorry, I mean "rootPath". It is needed to work with prpl-server, and adds the "/es6-bundled/" folder prefix here

So we need a function here, because it's added also when url is data:... will find a solution


Comments from Reviewable

manolo commented 6 years ago

Review status: 2 of 3 files reviewed at latest revision, 2 unresolved discussions.


src/expense-editor.html, line 77 at r2 (raw file):

Previously, manolo (Manuel Carrasco Moñino) wrote…
So we need a function here, because it's added also when url is `data:...` will find a solution

Done.


Comments from Reviewable

web-padawan commented 6 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

tomivirkki commented 6 years ago
:lgtm:

Reviewed 1 of 2 files at r1, 1 of 2 files at r2, 1 of 1 files at r4. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

web-padawan commented 6 years ago
:lgtm:

Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable

web-padawan commented 6 years ago

Reviewed 1 of 1 files at r4. Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.


Comments from Reviewable