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

Avoid trying to focus a detached element #6090

Merged
merged 1 commit into from
Jul 19, 2019
Merged

Conversation

Legioth
Copy link
Member

@Legioth Legioth commented Jul 18, 2019

All new tests except detachAfterFocus_nothingScheduled were passing also
before this fix.


This change is Reviewable

@project-bot project-bot bot added this to Review in progress in OLD Vaadin Flow ongoing work (Vaadin 10+) Jul 18, 2019
Copy link
Contributor

@caalador caalador 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: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @Legioth)


flow-server/src/main/java/com/vaadin/flow/component/Focusable.java, line 110 at r1 (raw file):

         * Make sure to call the focus function only after the element is
         * attached, and after the initial rendering cycle, so webcomponents can

How does this now make certain that the element is attached before calling focus?

All new tests except detachAfterFocus_nothingScheduled were passing also
before this fix.
Copy link
Member Author

@Legioth Legioth 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: 1 unresolved discussion, 0 of 1 LGTMs obtained (waiting on @caalador)


flow-server/src/main/java/com/vaadin/flow/component/Focusable.java, line 110 at r1 (raw file):

Previously, caalador wrote…
         * Make sure to call the focus function only after the element is
         * attached, and after the initial rendering cycle, so webcomponents can

How does this now make certain that the element is attached before calling focus?

That's handled by setTimeout in the JS snippet. I've clarified the comment.

Copy link
Contributor

@caalador caalador left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! all discussions resolved, 1 of 1 LGTMs obtained

OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Review in progress to Reviewer approved Jul 18, 2019
@denis-anisimov denis-anisimov merged commit 329b5f6 into master Jul 19, 2019
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from Reviewer approved to Done - pending release Jul 19, 2019
@denis-anisimov denis-anisimov deleted the focusDetachedElement branch July 19, 2019 05:43
@denis-anisimov denis-anisimov added this to the 2.0.5 milestone Jul 24, 2019
mehdi-vaadin pushed a commit that referenced this pull request Aug 9, 2019
All new tests except detachAfterFocus_nothingScheduled were passing also
before this fix.

(cherry picked from commit 329b5f6)
mehdi-vaadin pushed a commit that referenced this pull request Aug 9, 2019
All new tests except detachAfterFocus_nothingScheduled were passing also
before this fix.

(cherry picked from commit 329b5f6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
OLD Vaadin Flow ongoing work (Vaadin ...
  
Done - pending release
Development

Successfully merging this pull request may close these issues.

None yet

4 participants