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

Call renderer when first opened #110

Closed
wants to merge 1 commit into from
Closed

Call renderer when first opened #110

wants to merge 1 commit into from

Conversation

samiheikki
Copy link
Contributor

@samiheikki samiheikki commented Aug 30, 2018

Fixes #108


This change is Reviewable

@tomivirkki
Copy link
Member


test/vaadin-dialog_renderer-test.html, line 61 at r1 (raw file):

      });

      it('should not render the content of renderer function if the dialog is not opened', () => {

For me, this test is passing even with the current master

@tomivirkki
Copy link
Member

a discussion (no related file):
I think the problem is on the vaadin-overlay level and should be fixed there.

So the right fix would be to add "opened" in the list of observer method dependencies here: https://github.com/vaadin/vaadin-overlay/blob/master/src/vaadin-overlay.html#L348

...and then invoking "render()" https://github.com/vaadin/vaadin-overlay/blob/master/src/vaadin-overlay.html#L818 only when the overlay is opened (for the first time).


Copy link
Contributor Author

@samiheikki samiheikki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @samiheikki)

a discussion (no related file):

Previously, tomivirkki (Tomi Virkki) wrote…

I think the problem is on the vaadin-overlay level and should be fixed there.

So the right fix would be to add "opened" in the list of observer method dependencies here: https://github.com/vaadin/vaadin-overlay/blob/master/src/vaadin-overlay.html#L348

...and then invoking "render()" https://github.com/vaadin/vaadin-overlay/blob/master/src/vaadin-overlay.html#L818 only when the overlay is opened (for the first time).

Created vaadin/vaadin-overlay#120


@samiheikki samiheikki closed this Sep 3, 2018
@samiheikki samiheikki deleted the fix/renderer branch September 3, 2018 11:21
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.

None yet

2 participants