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

Bugfix/duplicated load of yjs #4542

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

luka-nextcloud
Copy link
Contributor

@luka-nextcloud luka-nextcloud commented Jul 17, 2023

📝 Summary

Reset value of window['__ $YJS$ __'] before Editor component is mounted to avoid console error Yjs was already imported.

🖼️ Screenshots

🏚️ Before 🏡 After
B A

🚧 TODO

  • ...

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@luka-nextcloud luka-nextcloud self-assigned this Jul 17, 2023
@luka-nextcloud luka-nextcloud added bug Something isn't working 3. to review labels Jul 17, 2023
@cypress
Copy link

cypress bot commented Jul 17, 2023

Passing run #11562 ↗︎

0 149 2 0 Flakiness 0

Details:

Bugfix/duplicated load of yjs
Project: Text Commit: e80e8b1eb5
Status: Passed Duration: 04:15 💡
Started: Aug 8, 2023 8:15 AM Ended: Aug 8, 2023 8:20 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@luka-nextcloud luka-nextcloud force-pushed the bugfix/duplicated-load-of-yjs branch from db21029 to 9661d6f Compare July 21, 2023 10:14
@juliusknorr
Copy link
Member

Small comments, but the general approach looks good :) Haven't tested yet though

@luka-nextcloud luka-nextcloud force-pushed the bugfix/duplicated-load-of-yjs branch from 9661d6f to e37da6f Compare July 24, 2023 14:10
@max-nextcloud
Copy link
Collaborator

@luka-nextcloud I just had a brief look at this - you marked three conversations with julius as resolved but only one of them seems to be addressed (the timeout). I don't understand why you resolved the other ones. When you resolve a conversation without changes to the code - could you add a brief comment explaining why? That would make it easier to understand what is going on.

Thanks! ❤️

@luka-nextcloud luka-nextcloud force-pushed the bugfix/duplicated-load-of-yjs branch from e37da6f to e80e8b1 Compare August 8, 2023 08:13
@luka-nextcloud
Copy link
Contributor Author

@max-nextcloud All of the comments are resolved now.

@luka-nextcloud luka-nextcloud force-pushed the bugfix/duplicated-load-of-yjs branch from e80e8b1 to 584ad58 Compare August 21, 2023 16:35
@max-nextcloud max-nextcloud force-pushed the bugfix/duplicated-load-of-yjs branch from 584ad58 to 94eca01 Compare August 29, 2023 17:25
@max-nextcloud
Copy link
Collaborator

/compile

Signed-off-by: nextcloud-command <[email protected]>
@max-nextcloud max-nextcloud merged commit f61f612 into main Aug 29, 2023
@max-nextcloud max-nextcloud deleted the bugfix/duplicated-load-of-yjs branch August 29, 2023 17:39
@Taikaschi
Copy link

I still have this error in my browser console after upgrade to 27.1.0 final.

@max-nextcloud
Copy link
Collaborator

I still have this error in my browser console after upgrade to 27.1.0 final.

As far as i can tell this has not been backported to 27. I'll trigger a backport - not sure if we want to backport but we can discuss that in the backport PR.

@max-nextcloud
Copy link
Collaborator

/backport 94eca01 to stable27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Prevent duplicate loading of yjs when workspace and viewer open text
5 participants