ckeditor / github-writer

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

Tooltips do not show up until the page is refreshed #375

Closed dufipl closed 1 year ago

dufipl commented 1 year ago

๐Ÿ“ Provide detailed reproduction steps (if any)

  1. Build GH Writer on ghw/epic/388-version-3521-bump branch
  2. Try to open a new issue.
  3. Hover toolbar buttons, if tooltips show up, go to the issue list and try to create the issue again.

โœ”๏ธ Expected result

Tooltips show up regardless of the cache.

โŒ Actual result

Tooltips do not show up until the page is refreshed tooltips issue


If you'd like to see this fixed sooner, add a ๐Ÿ‘ reaction to this post.

mlewand commented 1 year ago

Interesting, this issue should be in case reproducible in vanilla CKE5 - let's investigate it further.

Acrophost commented 1 year ago

More bugs most likely related to cache (occur when tooltips are not showing up):

In markdown (nostalgia) editing there is no preview or bottom bar for adding files from computer. Hard refresh/clearing cache fixes it just like tooltipsย 

mateuszzagorski commented 1 year ago

Bottom bar is not visible because body does not have the github-writer-loaded class and so the CSS rules we have are causing it to have display: none. Not sure yet why that's happening - investigating.

mateuszzagorski commented 1 year ago

Regarding the tooltips what I see is that inside the ck-body-wrapper we're missing a div with ck ck-balloon-panel ck-balloon-panel_arrow_n ck-balloon-panel_with-arrow ck-tooltip classes. ย 

In the recently added tooltipManager we have the bodyViewCollection that's set to first( TooltipManager._editors.values() ) and because of that we're targeting the editor that's no longer visible but it is still available via the TooltipManager._editors.values(). ย 

So in my opinion we have to update the ckeditor5-ui plugin in order to fix the issue.

You can replicate the similar behavior in ckeditor5:
- use the all-types manual test
- destroy the ClassicEditor
Without it tooltips will no longer be visible in all the other editors.

Additional notes:
- Clearing up the TooltipManager._editors set seems to fix the issue for GitHub Writer - but it does not work well for ckeditor5 itself so it needs to be investigated further at the source.

Dumluregn commented 1 year ago

I agree with @mateuszzagorski 's conclusions. I'll report an upstream issue in CKE5 repo and once it's resolved, we should go back here to ensure it fixed Writer.

Dumluregn commented 1 year ago

Reported in ckeditor/ckeditor5#12602.

mlewand commented 1 year ago

Closing as an upstream issue.

dufipl commented 1 year ago

@mlewand As the upstream issue is closed, I assume it should be fixed, but it's not, therefore I will reopen the issue.

Updating branch in reproduction steps to ghw/epic/388-version-3521-bump.

mlewand commented 1 year ago

This should be fixed with #417.