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

[#544] Added uliStore data structure, getAllTextNodes() and locateSlur() functions #597

Merged
merged 3 commits into from
Jul 19, 2024

Conversation

hardik-pratap-singh
Copy link
Contributor


name: uliStore v 1.0.0
about: Data structure preparation for efficient storage of slurs


Describe the PR
PR is related to the discussion done in following comments

Steps to test the PR
Code is added to plugin/src/transform-general.js

Screenshots

  • image

  • image

  • image

  • image

@aatmanvaidya aatmanvaidya self-requested a review July 19, 2024 04:37
aatmanvaidya
aatmanvaidya previously approved these changes Jul 19, 2024
@aatmanvaidya
Copy link
Collaborator

Changes required

  1. On Line 80 - let's not define the list of Slur words here, targetWords list can be an input of the locateSlur() function.
  2. On Line 130 - you can remove such commented instances of the calling the functions,
// getAllTextNodes(body);

this file should just contain the functions.
This way there is also no need to define body on line 79, you can remove that too

let body = document.querySelector("body");
  1. On Line 78 - you can initialise uliStore inside the getAllTextNodes() function, as the requirement of the function was that it takes in a node as input and returns uliStore in return.

@aatmanvaidya
Copy link
Collaborator

Hi @hardik-pratap-singh, everything looks good,

just one small thing, if I understand correctly, targetWords should also be an input of locateSlur(uliStore) right?

@hardik-pratap-singh
Copy link
Contributor Author

hardik-pratap-singh commented Jul 19, 2024

Hi @hardik-pratap-singh, everything looks good,

just one small thing, if I understand correctly, targetWords should also be an input of locateSlur(uliStore) right?

That depends where we are going to use this function, but for now I think it will be better to add it as input.
Also should I export these functions or not now ?

@aatmanvaidya
Copy link
Collaborator

no no, if there is no export for now that's fine,

things are looking good, merging now

browser-extension/plugin/src/transform-general.js Outdated Show resolved Hide resolved
browser-extension/plugin/src/transform-general.js Outdated Show resolved Hide resolved
browser-extension/plugin/src/transform-general.js Outdated Show resolved Hide resolved
@aatmanvaidya aatmanvaidya merged commit d197363 into tattle-made:main Jul 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants