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

fix: Properly emit ready event on conflicts with the editor API #4028

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Apr 6, 2023

📝 Summary

Fixes loading the editor integrated through the editor API when a file conflict occurs during the initial loading. In this case the ready event was never emitted on the root component that would pass the state to the notes app.

I'd love to have a more generic way there but haven't found a proper way to avoid emitting on both this and parent, so the second commit cleans up the emit handling in Editor.vue to make sure we always emit on both.

🏁 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

@cypress
Copy link

cypress bot commented Apr 6, 2023

2 flaky tests on run #9392 ↗︎

0 142 1 0 Flakiness 2

Details:

fix: Properly emit ready event on conflicts with the editor API
Project: Text Commit: 4f2f4cbcb3
Status: Passed Duration: 03:46 💡
Started: Apr 12, 2023 3:16 PM Ended: Apr 12, 2023 3:20 PM
Flakiness  sync.spec.js • 1 flaky test

View Output Video

Test Artifacts
Sync > recovers from a lost connection Output Screenshots
Flakiness  share.spec.js • 1 flaky test

View Output Video

Test Artifacts
Open test.md in viewer > Share a file with download disabled shows an error Output Screenshots

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

@juliusknorr
Copy link
Member Author

Also fixes this issue on Collectives as there the parent component is the ViewerComponent

@juliusknorr juliusknorr added bug Something isn't working 3. to review labels Apr 6, 2023
@juliusknorr
Copy link
Member Author

/backport to stable26

Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Thanks for tackeling this @juliushaertl. Code looks good to me 👍

@juliusknorr juliusknorr force-pushed the bugfix/noid/event-api branch from 9ae3314 to c5386b2 Compare April 12, 2023 12:03
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@mejo- mejo- force-pushed the bugfix/noid/event-api branch from c5386b2 to 94160a9 Compare April 12, 2023 14:49
@mejo-
Copy link
Member

mejo- commented Apr 12, 2023

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rich Text Mode: loading note issue
3 participants