Skip to content
This repository has been archived by the owner on Jan 26, 2023. It is now read-only.

rewrite web_ext with react #47

Open
gasolin opened this issue Mar 14, 2018 · 9 comments
Open

rewrite web_ext with react #47

gasolin opened this issue Mar 14, 2018 · 9 comments

Comments

@gasolin
Copy link
Collaborator

gasolin commented Mar 14, 2018

During porting back features in #46, I found the current popup html/js are pretty complicated and several issues stall the development speed

  • jquery dependency, which could be removed
  • spaghetti code: it makes code hard to test
  • fetchBounties are called twice (1. get all bounties then 2. getBounties again after page onload)
  • dead code (gitcoin tip related code are not functional)

Therefore I created an experiment repo with create-react-app to struct the popup page. The changes are

The repo is now at https://github.com/gasolin/web_ext which can build for chrome with yarn build command.

@owocki @KennethAshley how do you think if we replace chrome_ext with web_ext for the future version once its stable?

@owocki
Copy link
Contributor

owocki commented Mar 14, 2018

related => @kamescg did a react prototype of this thats quite nice! #31

i agree that the current chrome_Ext code is unsustainable.

https://github.com/gasolin/web_ext looks really nice.. is it ready for a regression test?

@gasolin
Copy link
Collaborator Author

gasolin commented Mar 15, 2018

@owocki I think web_ext functions are in parity with current chrome_ext now. Haven't add test yet but you can try by clone the repo https://github.com/gasolin/web_ext and run yarn install/yarn build (or npm install / npm run build).

Here's the screencast.

Imgur

Some differences are I use bootstrap 4 css from npm module and comment out the header/bot images.
You can add header/side bot image back easily.

I checked @kamescg branch and there are some good things like theming 👍 . Though I'd like to have less webpack configuration which brings burden when upgrading from version to version yearly.

@owocki
Copy link
Contributor

owocki commented Mar 15, 2018

wow, this looks like major progress!!

i think we might want to tweak the styling on the gitcoin pop-in, but other than that this looks great!

@owocki
Copy link
Contributor

owocki commented Mar 15, 2018

want to start a PR and we can do some QA over there?

as for the repo name, maybe well change it from chrome_ext to web_ext :)

@gasolin
Copy link
Collaborator Author

gasolin commented Mar 16, 2018

@owocki sure, please check #48
I also added some basic tests to make sure test framework works

@gasolin
Copy link
Collaborator Author

gasolin commented Apr 23, 2018

Here are tasks breakdown as future browser_ext development plan proposal

Overall

Content

Popup

@gasolin
Copy link
Collaborator Author

gasolin commented Apr 23, 2018

@owocki @mbeacom above are recent tasks in mind.
All tasks are filed with issue number. Please take a look and add your comment, thanks

@owocki
Copy link
Contributor

owocki commented Apr 25, 2018

Detect if inject element already exists #21 Small
Support web extension (Firefox) #1 Medium
Use fetch for all queries #42 Small
Align issue tag style #50 Small

i think these are the ones were really excited about internally

@gasolin
Copy link
Collaborator Author

gasolin commented May 5, 2018

Hi @owocki current PR did the rewrite and fixed #42 #50, do you think if we can merge the PR then do the rest work on the new base?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants