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

first commit - a lot works but a little sucks #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Hamzaisongit
Copy link

main things i've used till now..

  • appscript for accessing and manipulating Gsheet
  • levenshtine distance algorithm for stock-symbol matching

Not completed 100% yet, just a few things to get around with.

Comment on lines +20 to +26
fetch('https://script.google.com/macros/s/AKfycbxJPjCcowBQS124lLZfmw1ItprLtxNx7MiAmTgsdcp_dpTazJJVM7zEOrM_KHPSoib4/exec', {
method: 'GET',
mode: 'cors'
}).then(res => res.json()).then((data) => {
console.log(data)
symbolData = data
})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not store it in user's local storage, rather refetching in background every time, if there is update we can update it

Copy link
Author

@Hamzaisongit Hamzaisongit Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i had the idea,
just wanted the base logic to work as of now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than using raw localstorage use something like dexie-db. Your life will be much easier.

Comment on lines +3 to +10

console.log('This is the background page.');

console.log('Put the background scripts here.');

console.log('new line by hamza');

//fetch('http://localhost:5000').then((res) => res.json()).then((res) => { console.log(JSON.stringify(res)) })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots of console log, remove them, not good in production

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yet to remove all this.

Comment on lines 32 to 41

function func(clickedSymbol) {

if (!symbolData) return;

const nearbyIndexes = symbolData.map((i) => {
const arrayOfSimilarSymbols = i.split(",")
const nearbyIndex = distance(clickedSymbol, closest(clickedSymbol, arrayOfSimilarSymbols.map((i) => {
[...Array(25 - i.length).keys()].forEach(() => i += '*')
return i
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really bad name for function func, give it something meaningful, that is easy to understand by you after 1 month, cause you will forget what this code does

Copy link
Author

@Hamzaisongit Hamzaisongit Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay

const nearbyIndexes = symbolData.map((i) => {
const arrayOfSimilarSymbols = i.split(",")
const nearbyIndex = distance(clickedSymbol, closest(clickedSymbol, arrayOfSimilarSymbols.map((i) => {
[...Array(25 - i.length).keys()].forEach(() => i += '*')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you thought what would happen if index goes in negative, Array(-12) ?

try it in your browser's console

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as of now, i have assumed the maximum symbol size to be 25 characters, so..

Comment on lines +48 to +61

nearbyIndexes.forEach((i, n) => {
if (i == "") return
nearbyIndexesObjs[n] = { [symbolData[n]]: i }
})


console.log(clickedSymbol)
nearbyIndexesObjs.sort((a, b) => Object.values(a)[0] - Object.values(b)[0])
console.log(nearbyIndexesObjs[0], nearbyIndexesObjs[1], nearbyIndexesObjs[2], nearbyIndexesObjs[3], nearbyIndexesObjs[4])


}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment for better readability of the code and it's logic

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do it,

Comment on lines 29 to 31
const clickedSymbol = anchor.textContent.toLocaleLowerCase().replace(/[ .]/g, "")

chrome.runtime.sendMessage({ msg: clickedSymbol }, () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is possibility of anchor being undefined which can lead to error "cannot read properties of undefined(reading textContent)"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anchor tag being undefined : i think it has been already handled at beginning of the function by that if condition

@@ -40,7 +40,7 @@ const SnippetList = () => {
return (
<div className={`w-full h-full font-sans ${isDarkMode ? 'bg-gray-900' : 'bg-gray-100'} flex flex-col`}>
<div className="flex justify-between items-center p-4">
<h1 className={`text-xl font-semibold ${isDarkMode ? 'text-gray-200' : 'text-gray-800'}`}>Snippets</h1>
<h1 className={`text-xl font-semibold ${isDarkMode ? 'text-gray-200' : 'text-gray-800'}`}>these are Snippets</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use capital case if needed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay,

Comment on lines +5 to +11
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>

<body>
<h1>custom options page</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't get the utility of this folder cause your server.js file is at the root level

Copy link
Author

@Hamzaisongit Hamzaisongit Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was only for experimenting with the DOM selectors and just to see if everything works fine while injecting the script on a similarly structured HTML page,

i'll put it inside gitIgnore, doesn't need to be pushed along ig

Copy link
Member

@krishna-404 krishna-404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a video of using it... Need to see that to understand what is happening here...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

levenstein distance is some ways (not-exactly) non-determenistic... what happens when 2 symbols match to this... we will mostly need exact match... but we can start with this for now & make it better later...

onClickHandler(message.msg)
})

function onClickHandler(clickedSymbol) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what onclick handler? needs to be more verbose on what this function is doing... could be onSymbolClick

Comment on lines +20 to +26
fetch('https://script.google.com/macros/s/AKfycbxJPjCcowBQS124lLZfmw1ItprLtxNx7MiAmTgsdcp_dpTazJJVM7zEOrM_KHPSoib4/exec', {
method: 'GET',
mode: 'cors'
}).then(res => res.json()).then((data) => {
console.log(data)
symbolData = data
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rather than using raw localstorage use something like dexie-db. Your life will be much easier.

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.

4 participants