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

iframe example - use an onLoad callback instead of the ref #4758

Merged
merged 3 commits into from
Jan 10, 2022
Merged

iframe example - use an onLoad callback instead of the ref #4758

merged 3 commits into from
Jan 10, 2022

Conversation

adrid
Copy link
Contributor

@adrid adrid commented Jan 2, 2022

Description
Makes the iframe example work on the Firefox and keeps it working on other browsers. It changes using the ref on the iframe to an onLoad event solution

Issue
Fixes: #4285

Example
https://www.slatejs.org/examples/iframe on the Firefox

Context
Firefox resets the iframe content on the load event. I've tested on some older versions of the browser and it keeps happening too. Here is an issue on the react repo facebook/react#22847

Checks

  • The new code matches the existing patterns and styles.
  • The tests pass with yarn test.
  • The linter passes with yarn lint. (Fix errors with yarn fix.)
  • The relevant examples still work. (Run examples with yarn start.)

@changeset-bot
Copy link

changeset-bot bot commented Jan 2, 2022

⚠️ No Changeset found

Latest commit: d1602fc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

site/examples/iframe.tsx Outdated Show resolved Hide resolved
site/examples/iframe.tsx Outdated Show resolved Hide resolved
@nemanja-tosic
Copy link
Contributor

Just to make sure, it would not be possible to pass in onLoad load anymore. Is that intentional?

@adrid
Copy link
Contributor Author

adrid commented Jan 9, 2022

Just to make sure, it would not be possible to pass in onLoad load anymore. Is that intentional?

Yes. I don't think this is needed for this example.

@dylans dylans merged commit e3a325f into ianstormtaylor:main Jan 10, 2022
@dylans
Copy link
Collaborator

dylans commented Jan 11, 2022

@adrid, this change has broken our integration test. Any chance you have time to update the test to reflect the change in the example?



  iframe editor
    1) should be editable


  0 passing (10s)
  1 failing

  1) iframe editor
       should be editable:
     TestingLibraryElementError: Timed out retrying after 4000ms: Unable to find an accessible element with the role "textbox"

There are no accessible roles. But there might be some inaccessible roles. If you wish to access them, then set the `hidden` option to `true`. Learn more about this here: https://testing-library.com/docs/dom-testing-library/api-queries#byrole
      at Object.getElementError (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:1764:17)
      at eval (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:2729:25)
      at eval (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:2701:24)
      at eval (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:2754:25)
      at baseCommandImpl (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:477:16)
      at commandImpl (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:480:40)
      at getValue (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:504:23)
      at resolveValue (http://localhost:3000/__cypress/tests?p=cypress/support/index.ts:544:35)




  (Results)

  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ Tests:        1                                                                                │
  │ Passing:      0                                                                                │
  │ Failing:      1                                                                                │
  │ Pending:      0                                                                                │
  │ Skipped:      0                                                                                │
  │ Screenshots:  1                                                                                │
  │ Video:        true                                                                             │
  │ Duration:     10 seconds                                                                       │
  │ Spec Ran:     examples/iframe.ts                                                               │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘

@adrid
Copy link
Contributor Author

adrid commented Jan 11, 2022

@dylans when I created the pull request it was not passing this test and I noticed that later - the same thing what you pointing out.

I've already fixed it here in the next commit 87a8604

I've tested on the main branch (as all those commits are merged) and seems to be okay so maybe you referring to some check before I fixed that?

DougReeder pushed a commit to DougReeder/slate that referenced this pull request Apr 3, 2022
…aylor#4758)

* use an onLoad callback instead of the ref

* expect body to not be null on the iframe test

* remove onLoad prop
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.

Slate does not work in Firefox Iframes
3 participants