UX of add slur and Crowdsource in single button

[anshuman-rai-27]

issue no : #423
Integrated both the button in single button with both functionality.

[ghost]

👇 Click on the image for a new way to code review

Legend

[aatmanvaidya]

@anshuman-rai-27 there are some changes required in this PR.

currently, you have just changed the frontend of this issue, like with the code change you made, I wont be able to see "Crowdsource Slur Word" when I right click on a word.

But the issue was to merge both the functionalities - "Add Slur to List" and "Crowdsource Slur Word" together,

what this means is - if you look carefully, the background.js file sends message to the content-script.js file.
So say when a user clicks on "Crowdsource Slur Word", the background.js file sends a message to content-script.js saying, the user wants to crowdsource the word. After which, the content-script.js file triggers the REST API call and hence a word is crowdsoucre.

For this issue, you will also have to modify the content-script.js file to make sure in one command both the action's are triggerd.

I hope this is easy to follow, if not, please let me know! I can explain further.

[anshuman-rai-27]

@aatmanvaidya i have made changes in content-script.js file please review and guide me asap as hacktoberfest is at an end

[aatmanvaidya]

Hello @anshuman-rai-27, please give me some time to review, as I will have to test it across browsers..

Just curious, were you able to setup Uli locally for development and test this thing out?

[aatmanvaidya]

Hello @anshuman-rai-27 , I have reviewed your PR some changes are required.

1. You can remove the commented code in both the files. Additionally, it will be great if you could indent the code properly this would lead to better readability, especially in open-source projects where multiple contributors are involved. This would make it easier for others to understand and work with the code.
2. If you notice carefully, the background.js file is sending the same message (i.e. the word which was right clicked) in the requests - SLUR_ADDED and CROWDSOURCE_SLUR_WORD. So you don't have to send the same thing twice, just send any one of the following to content-script.js
3. After that you don't have to have this if statement

if (request.type === 'SLUR_ADDED' || request.type === 'CROWDSOURCE_SLUR_WORD') {

just check for the one request you are sending from background.js
4. Next, in the same if statement in content-script.js
you don't need to declare two variables for the same content you are receiving from background.js. This will help keep things more cleaner and less error prone
5. In case you want to test this out locally, reach out to me, I can help you, some modifications are required in the manifest.json file to make this feature work

[anshuman-rai-27]

@aatmanvaidya soory i am unable to run this locally made the changes as you guided so guid me for the changes needed in manifest.json

[aatmanvaidya]

yes, were you able to get the docker up and running and load the extension into any browser?

if you are developing on chrome, in manifest.json under "permissions" add this http://localhost:3000/*

so now your "permissions" in manifest.json will look like this

    "permissions": [
        "storage",
        "contextMenus",
        "https://ogbv-plugin.tattle.co.in/*",
        "http://localhost:3000/*"
    ],

[anshuman-rai-27]

@aatmanvaidya soory but i am unable to run docker on my system as my system crashes every time i run docker so i am not able to test the changes , if possible i test it and suggest changes

[aatmanvaidya]

Hi @anshuman-rai-27 , thank you for your efforts and work on the PR - we have resolved this issue in this PR - #484