ipfs-companion icon indicating copy to clipboard operation
ipfs-companion copied to clipboard

feat: reload failed IPFS tabs when API becomes available

Open whizzzkid opened this issue 3 years ago • 6 comments
trafficstars

Closes https://github.com/ipfs/ipfs-companion/issues/1010

In this PR:

  • I added support for reloading failed tabs when API is available again, by:
    • By statically checking for failed tabs which belongs to local gateway instead of tracking all URLs.
    • This reduces the complexity for tracking failed websites in content_scripts.
    • More details on the implementation can be found in the relevant reloader.
  • I am also introducing a plugin mechanism that can register different reloader classes, this provides decent segregation of validation logic and also allows us to have reusable code for reloading.
  • Added relevant unit tests for 100% coverage.
  • Also updated the yarn.lock file and added a matching .editorconfig to configure IDEs.

Screencast:

https://user-images.githubusercontent.com/1895906/186346687-5de9caec-c4fc-4897-91cc-bed07990ba9d.mp4

whizzzkid avatar Aug 24 '22 06:08 whizzzkid

Thank you for submitting this PR! A maintainer will be here shortly to review it. We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the rest) and in its best form. Follow the code contribution guidelines if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment. Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on any missing things and potentially assigning a reviewer for high priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within seven business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. We are very grateful for your contribution!

welcome[bot] avatar Aug 24 '22 06:08 welcome[bot]

@SgtPooki looks like the builds are failing on windows-latest I created a PR to address that: #1094 can you please try that?

whizzzkid avatar Sep 01 '22 05:09 whizzzkid

@lidel @SgtPooki this is ready for re-review:

  • [x] rebasing this to include fix from https://github.com/ipfs/ipfs-companion/pull/1094 (which already landed in main branch)
  • [x] improving URL check to also work for subdomain gateways (details inline)
  • [x] (optional) perhaps we could avoid title matching by leveraging onErrorOccurred? (details inline not a blocker, could be descoped and filled as an issue for future improvement, lmk if you prefer that): #1097

Build is now 🟢 https://github.com/whizzzkid/ipfs-companion/actions/runs/3033417199

❤️

whizzzkid avatar Sep 11 '22 22:09 whizzzkid

@SgtPooki : is there any other feedback, or can this be merged?

BigLep avatar Sep 19 '22 12:09 BigLep

GUI triage: Lidel is going to give a look at this. We need to fix companion build pipeline before merging this

SgtPooki avatar Sep 19 '22 17:09 SgtPooki

@SgtPooki are the build pipelines broken again? I fixed the companion pipeline in https://github.com/ipfs/ipfs-companion/pull/1094

whizzzkid avatar Sep 19 '22 17:09 whizzzkid

@SgtPooki are the build pipelines broken again? I fixed the companion pipeline in #1094

Sorry we missed that =) Gave my approval but I've got very little experience in this repo so I'm leaving the PR open for @lidel to handle when he gets to it. I'd ping him Monday if it's not merged by then

SgtPooki avatar Sep 24 '22 00:09 SgtPooki

GUI triage: Lidel is going to give a look at this. We need to fix companion build pipeline before merging this

SgtPooki avatar Sep 26 '22 16:09 SgtPooki