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

GH Writer doesn't load for comment editing on touch devices and poor network conditions #285

Open
mlewand opened this issue Nov 4, 2021 · 0 comments
Labels
bug Something isn't working
Milestone

Comments

@mlewand
Copy link
Contributor

mlewand commented Nov 4, 2021

Steps to reproduce touch

  1. Use a device with touch screen capabilities (best a notebook with a touch screen).
  2. Open an issue.
  3. Touch the "More options…" (ellipsis) button for a given comment.
  4. Touch the edit option.

Expected

GH writer gets loaded.

Actual

Regular editor is enabled, you can't turn it into GH writer.

Steps to reproduce poor network

  1. Use devtools and set the network to slow 3G network.
  2. Open an issue.
  3. Click on the "More options…" (ellipsis) button for a given comment.
  4. Click the edit option.

Expected

GH writer is loaded.

Actual

If you don't give it too much time it will fail to load GH writer.


It's a race condition.

While working on functional tests (#281) we noticed that the code listens for clicks in .timeline-comment-action element and then tries to find the .js-comment-edit-button button.

However, the .js-comment-edit-button button is loaded asynchronously - but GH is smart and is preloading the contents as you move your cursor over the more button (but before you click it). If it has fetched the content before you click, the content is precached - so it renders it asynchronously.

That's why we didn't see it in 99% cases.

Possible solution

I believe that the solution will be dead simple. Instead of finding this .js-comment-edit-button button we should match the "More options…" button. You can match it by querying actionsButton.closest( '[role="button"].timeline-comment-action' ) and from there you can query the form.js-comment-update in parents chain.

Note, we can't match just details element as reactions and one more UI element also use details.

@mlewand mlewand added the bug Something isn't working label Nov 4, 2021
@mlewand mlewand added this to the 1.8.2 milestone Nov 8, 2021
@mlewand mlewand modified the milestones: 1.9.0, 1.9.1 Mar 8, 2022
@mlewand mlewand modified the milestones: 1.9.1, 1.9.2 Aug 12, 2022
@mlewand mlewand modified the milestones: 1.10.1, 1.10.2 Nov 17, 2022
@mlewand mlewand modified the milestones: 1.10.2, 1.10.3 Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants