Closed Comandeer closed 1 year ago
@Dumluregn let's have it reviewed either by you or me.
I've tested this briefly, and can confirm that it is working way better now.
However there are still cases in which the editor does not load for me:
- When I'm opening PR for the first time, the comment editor at the bottom does not update. It starts to work after refreshing the page.
- Trying to edit any of the comments (works when you edit the same comment again).
- Creating new PR (works after refreshing).
@mateuszzagorski, I can only reproduce the issue with editing comments. Indeed, the editor is then partially loaded – yet it doesn't seem to fix itself after editing the same comment again. However, I can't reproduce other issues you raise. Could you share some more details?
Sure @Comandeer. I am experiencing the issues when using Chrome (v 104.0.5112.79 ).
On Firefox (v 103.0.2) I was unable to replicate even the editing comments problem.
1. Opening PR for the first time:
- Go to the 'pull requests' tab https://github.com/ckeditor/github-writer/pulls
- Click on any of the available PR's
- Scroll all the way down, and what I see is:
After refreshing it's working as expected.
2. Creating a new PR
- Go to the 'pull requests' tab -> https://github.com/ckeditor/github-writer/pulls and simply proceed with creating a new PR
And what I can see is:
Similarly to the previous issue - when refreshed, it's working as expected.
I can't reproduce the issue with creating new PR but I'm able to reproduce the issue with navigating to PR. It's present only at the first navigation to the PR, then it disappears.
I conducted a research on how this PR improves the loading of GH Writer in different scenarios.
master
branch. CodeEditor
).The results are in the table below. To sum up:
I think that this PR is a great improvement on its own and it's worth to release a new version once we merge it.
(the table below has 5 columns - you may need to scroll it horizontally)
Tested item | Exemplary URL | Fail ratio | Comment | Worked before? |
---|---|---|---|---|
comment on the issue page | #346 | 0 | mostly yes | |
new issue | https://github.com/ckeditor/github-writer/issues/new?assignees=&labels=bug&template=1-bug-report.md&title= | 0 | very bad, each time a new issue is created GHW doesn't load until refreshed | |
edit comment | https://github.com/ckeditor/github-writer/issues/347#issue-1331726626 | 30% | doesn't work only if you click "edit" button before the whole page is loaded (see zenhub or editor in new comment field) | same as after change |
new comment under pull request | ckeditor/ckeditor5#12254 | 0 | very bad, each time you visit a PR GHW doesn't load | |
new comment under pull request - first visit | ckeditor/ckeditor5#12254 | 100% | confirmed by Tomek and Mateusz - https://github.com/ckeditor/github-writer/pull/348#issuecomment-1210758285 | no |
new PR that can be automatically merged | https://github.com/ckeditor/ckeditor5/compare/master...ck/epic/11857-table-resize-improvements | 0 | even if clicked "Create pull request" before full page load | yes |
new PR that can't be automatically merged | https://github.com/ckeditor/github-writer/compare/master...t/188 | 100% | go to https://github.com/ckeditor/github-writer/compare, select t/188 and "Create pull request" | no |
new review comment | https://github.com/ckeditor/ckeditor5/pull/12254/files | 10% | only the first time (at all, not at each PR) | 50/50 |
edito non-markdown file | https://github.com/ckeditor/github-writer/edit/master/.gitignore | 100% | only markdown files are supported - by design | no |
edit markdown file | https://github.com/ckeditor/github-writer/edit/master/README.md | 0 | only markdown files are supported - by design | yes |
wiki page | https://github.com/ckeditor/ckeditor5/wiki/Home/_edit | 100% | no | |
saved replies | https://github.com/settings/replies | 100% | looks like GHW tries to load and resigns | no |
commit comment | ckeditor/ckeditor5@bcf3d44 | 0 | worked 30% of times | |
milestone | https://github.com/ckeditor/ckeditor5/milestones/55/edit | 0 | 50/50 | |
releases | https://github.com/ckeditor/ckeditor5/releases/new | 0 | yes | |
edit saved reply | https://github.com/settings/replies/81985/edit? | 0 | yes | |
release edit | https://github.com/ckeditor/ckeditor5/releases/edit/v35.0.1 | 100% | no |
Ok, since we have a confirmation that it indeed improves many cases - let's merge it.
The GHW's router listened to pjax events to detect when it should destroy the old editors and initialize new ones. However, it seems that GH no longer uses the pjax and it uses Turbo instead. I've replaced the pjax events with their Turbo equivalents:
pjax:start
→turbo:before-visit
,pjax:end
→turbo:render
.Thanks to that, the navigation between pages is correctly detected and handled and GHW is once again loading correctly.
Closes #337.