search

ckeditor / github-writer

GitHub Writer - WYSIWYG Rich-Text Editor for GitHub, powered by CKEditor.
https://ckeditor.com/
Other
376 stars 61 forks source link

Updated failing test after recent ckeditor5 version bump #395

Closed mateuszzagorski closed 1 year ago

mateuszzagorski commented 1 year ago

Focus is called twice on opening a dropdown so one of the tests had to be updated. Closes #363.

mateuszzagorski commented 1 year ago

Focus is being called 2 times:
Once from within the savedreplies plugin -> https://github.com/ckeditor/github-writer/blob/master/src/app/plugins/savedreplies.js#L48-L49
and it seems that the second time because of this method from the utils -> https://github.com/ckeditor/ckeditor5/blob/master/packages/ckeditor5-ui/src/dropdown/utils.ts#L418-L431

The savedreplies plugin wasn't updated at all for a long time - however we've done some major changes to the ui between 34.2.0 -> 35.1.0 or newer. I didn't go as much into details what & why but clearly the changes done in utils are causing this and I don't think there is much we can do.

Not sure how to properly add this information to the code - possibly just updating the mentioned ticket with the description what happend would be sufficient, wdyt?

mlewand commented 1 year ago

Thanks for checking this @mateuszzagorski! So what I see here is that now RepliesListView#focus() is called twice due to upstream changes.

So we're effectively duplicating the upstream code. In this case we should just remove the old code and let the upstream do its work.

Let's keep the test so that it acts as a integration/smoke test.

I'll adjust the PR for that.