p2plabsxyz / dscan

A decentralized storage and file-sharing tool that uploads content to IPFS and generates decentralized QR codes.
https://chromewebstore.google.com/detail/dscan-own-your-identity-o/idpfgkgogjjgklefnkjdpghkifbjenap
MIT License
38 stars 26 forks source link

Now builds for both Firefox and Chromium #24

Closed anjannair closed 2 years ago

anjannair commented 2 years ago

Related Issue (if any)

No open issue

Describe the add-ons or changes you've made

Added support for Firefox with this PR and updated all packages to the latest versions without breaking the extension.

Just like how it is mentioned in the documentation, to build the Chromium extension you need to run this - npm run build For Firefox you just run this - npm run buildFF

Type of change

How Has This Been Tested?

Tested the Firefox build on their stable build v103.0 (64-bit)

Checklist:

akhileshthite commented 2 years ago

Related Issue (if any)

No open issue

Describe the add-ons or changes you've made

Added support for Firefox with this PR and updated all packages to the latest versions without breaking the extension.

Just like how it is mentioned in the documentation, to build the Chromium extension you need to run this - npm run build For Firefox you just run this - npm run buildFF

Type of change

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Code style update (formatting, local variables)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] This change requires a documentation update

How Has This Been Tested?

Tested the Firefox build on their stable build v103.0 (64-bit)

Checklist:

  • [x] My code follows the "contribution guidelines" of this project.
  • [x] I have performed a self-review of my own code.
  • [x] I have commented my code, particularly wherever it was hard to understand.
  • [x] My changes generate no new warnings.
  • [x] Any dependent changes have been merged and published in downstream modules.

Thanks! Will review soon.

anjannair commented 2 years ago

Issue #25 will be faced here btw

akhileshthite commented 2 years ago

Hey @anjannair, I'm aware of the issue #25, but I'm facing different issues.

I faced the 2 following issues in this PR:

MacOS Mozilla Firefox (104.0.1)

1. The extension closes after clicking on the upload button.

After clicking on the upload button, it opens the file selector window (not shown in the below gif because it's not covering the whole window), and after selecting the files It's not publishing to IPFS (checked on web3.storage dashboard).

ezgif com-gif-maker

2. Horizontal and vertical scroll bar.

Screenshot 2022-08-31 at 10 50 09 PM

What is your browser version? can you show a working gif?

anjannair commented 2 years ago

@akhileshthite the main branch only has the Firefox build ready. The firefox branch however is a few commits ahead of the main branch consisting of the pop up solution. Do you want me to merge it into my main branch? Or do you want to test it before I merge it with my main branch

akhileshthite commented 2 years ago

@akhileshthite the main branch only has the Firefox build ready. The firefox branch however is a few commits ahead of the main branch consisting of the pop up solution. Do you want me to merge it into my main branch? Or do you want to test it before I merge it with my main branch

Screenshot 2022-09-01 at 3 09 05 PM

akhileshthite commented 2 years ago

@akhileshthite the main branch only has the Firefox build ready. The firefox branch however is a few commits ahead of the main branch consisting of the pop up solution. Do you want me to merge it into my main branch? Or do you want to test it before I merge it with my main branch

  • I'm dealing with this extension crashing issue (latest dscan update without your firefox branch solution) on firefox browser only, and not on chromium browsers (chrome, brave, etc.). Are you facing this Extension loses focus on file/folder upload #25 issue in chromium browsers? If so, then please mention your browser version and screen recording.
  • I tested your firefox branch, we don't want the Extension loses focus on file/folder upload #25 fix for normal build npm run build (chromium browsers).
  • Even on the firefox, I feel like opening a new window after clicking on the launch button would lead to a bad UX, can you try to fix the issue by keeping the old UI?

Screenshot 2022-09-01 at 3 09 05 PM

  • Don't change the naming. Screenshot 2022-09-01 at 6 07 24 PM
  • Let's keep a single main branch, so it would be easier for development and docs (merge your firefox with your main branch, so you don't need to make another PR for this issue).
  • Follow conventional commits guidelines as mentioned in the CONTRIBUTING.md.

@anjannair ?

anjannair commented 2 years ago

@akhileshthite please give me a day or 2 to respond. Am in the middle of something hence can't reply

akhileshthite commented 2 years ago

@akhileshthite please give me a day or 2 to respond. Am in the middle of something hence can't reply

Sure.

anjannair commented 2 years ago

@akhileshthite honestly I just thought I'd help and contribute. I tried recording my screen but Linux breaks the recording every time. I'll instead type out the following - Open Chrome (the latest stable version), try to upload a file/ folder and get the link.

You'll understand what I am talking about.

Secondly, I didn't understand what you meant by this. You don't want to fix the out-of-focus issue for Chromium users????

Third, there is no solution to fix the out-of-focus issue without changing the UI. If you want to keep it the same you will face the before-mentioned issue.

If you want to fix these issues I believe my PR should be the way to go. If you still believe otherwise please feel free to close this pr 👍🏽

akhileshthite commented 2 years ago

@akhileshthite honestly I just thought I'd help and contribute. I tried recording my screen but Linux breaks the recording every time. I'll instead type out the following - Open Chrome (the latest stable version), try to upload a file/ folder and get the link.

You'll understand what I am talking about.

Secondly, I didn't understand what you meant by this. You don't want to fix the out-of-focus issue for Chromium users????

Third, there is no solution to fix the out-of-focus issue without changing the UI. If you want to keep it the same you will face the before-mentioned issue.

If you want to fix these issues I believe my PR should be the way to go. If you still believe otherwise please feel free to close this pr 👍🏽

Google Chrome Latest Version 106.0.5249.91, it works ideally on Mac and Windows OS. Also, works ideally on brave, opera, and edge browsers.

Soon I'll test it on Linux OS, if you're dealing with any problems "YOU SHOULD" show them here. Your screen recorder is not working is not my problem, record it using a phone or something.

Please mention "every" detail in your PRs or issues. In your #25 issue, you didn't mention anything about browser, its version or OS. It's a really bad open-source practice.

akhileshthite commented 2 years ago

@akhileshthite the main branch only has the Firefox build ready. The firefox branch however is a few commits ahead of the main branch consisting of the pop up solution. Do you want me to merge it into my main branch? Or do you want to test it before I merge it with my main branch

  • I'm dealing with this extension crashing issue (latest dscan update without your firefox branch solution) on firefox browser only, and not on chromium browsers (chrome, brave, etc.). Are you facing this Extension loses focus on file/folder upload #25 issue in chromium browsers? If so, then please mention your browser version and screen recording.
  • I tested your firefox branch, we don't want the Extension loses focus on file/folder upload #25 fix for normal build npm run build (chromium browsers).
  • Even on the firefox, I feel like opening a new window after clicking on the launch button would lead to a bad UX, can you try to fix the issue by keeping the old UI?

Screenshot 2022-09-01 at 3 09 05 PM

  • Don't change the naming. Screenshot 2022-09-01 at 6 07 24 PM
  • Let's keep a single main branch, so it would be easier for development and docs (merge your firefox with your main branch, so you don't need to make another PR for this issue).
  • Follow conventional commits guidelines as mentioned in the CONTRIBUTING.md.

I've explained clearly here. Your current PR is changing the whole UI/UX of the extension for all browsers (which is poor).