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

NUX: Avoid popover refresh on tip mount #7748

Merged
merged 2 commits into from
Jul 9, 2018
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Jul 6, 2018

Closes #6956

This pull request seeks to resolve an issue where end-to-end tests frequently fail due to DotTip mount behavior to refresh its popover positioning. This was meant to be resolved in #7544 and #7493, yet persists. I suspect the bug is due to the fact that isVisible changes between the time the setTimeout is scheduled and when the callback is reached. The approach here intends to remove the setTimeout altogether, resolving the original issue with positioning by ensuring that the post title is rendered immediately. This requires that we change the default behavior of PostTypeSupportCheck to assume true if the post type is not yet known. I expect that this is probably the majority case anyways. This may also become a non-issue anyways once #7546 is merged, as the post type data is immediately available, but due to the behavior of resolvers being asynchronous, it doesn't take effect during the initial render.

Testing instructions:

  1. Clear local storage
    • Type localStorage.clear() in browser console.
  2. Refresh the editor page
  3. Note that the DotTip is positioned correctly

Ensure end-to-end tests pass:

npm run test-e2e

Ensure unit tests pass:

npm run test-unit

@aduth aduth added the [Feature] NUX Anything that impacts the new user experience label Jul 6, 2018
@aduth aduth requested a review from noisysocks July 6, 2018 14:18
Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

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

Sounds like a good compromise. Code LGTM—thanks for adding docs and tests!

@aduth aduth merged commit c85e3f9 into master Jul 9, 2018
@aduth aduth deleted the update/default-supports branch July 9, 2018 12:20
@gziolo gziolo added this to the 3.3 milestone Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] NUX Anything that impacts the new user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent e2e test failures
3 participants