tattle-made / kosh-v2

3 stars 4 forks source link

Bugfix - copy not working on firefox #64

Closed RishavT closed 2 years ago

RishavT commented 2 years ago

Fixes tattle-made/kosh-v2#63 (hopefully)

I can't access the tokens page, might need some help on that front - and until then I'm finding it tough to finish the following TODOs I had in mind:

  1. Test it
  2. Write the code in pure typescript and follow other conventions (I will feel more comfortable doing this if I can test it)
  3. Move the clipboard loading code to a more generic file, not tokens.js. This is debatable. Keeping it in tokens.js makes the rest of the website lighter.

Feel free to test / modify / incorporate a version of this bugfix if giving access seems tricky at the point and this is important :)

dennyabrain commented 2 years ago

Thanks @RishavT ! I might direct you to some docs to ensure you can actually test it on your side in a few days. Just one thought from a cursory review of the code. The conditional loading is happening at the ListItem component. Wouldn't this lead to multiple loads of the script? Would it make more sense to move it up one level ? So it loads once for the whole page as opposed to each list item.

dennyabrain commented 2 years ago

@RishavT there is a seeder script in the project that will just add enough data (few users, specifically) for you to be able to login and try out things. its here - https://github.com/tattle-made/kosh-v2/blob/main/src/backend/core/database/seeders/20211213080231-init-user-and-datasource.js.js

It uses the sequelize ORM. you can read up just this page to grok it - https://sequelize.org/docs/v6/other-topics/migrations/

I am noting down the "just enough" steps to run the script regardless. cd into the dir with .sequelizerc in it : cd kosh-v2/src/backend/core/database To run the seeder : npx sequelize-cli db:seed --seed 20211213080231-init-user-and-datasource.js.js

caveat : I just noticed that the seeder script has 2 .js in it. that's unintentional. Don't be thrown off by it :)

RishavT commented 2 years ago

Awesome, thanks @dennyabrain ! Will test and make this PR ready for review. And yes, I'll move the code to load the script up one level (although it ideally wouldn't load it multiple times - I messed up the if condition, I'm checking just for navigator.clipboard whereas I should have checked for both window.customClipboard as well as navigator.clipboard and if both are null, only then load the script). Moving it up still makes sense :)

dennyabrain commented 2 years ago

Oh right, both window and navigator are from the global scope. Missed that.

RishavT commented 2 years ago

So I've been following these steps, need some help:

  1. Trying to run the docker compose stack locally - it needs some env file I guess - open /home/rishav/Documents/personal/kosh-v2/src/backend/development.env: no such file or directory
  2. After this I'll use the sql file https://tattle-media.s3.amazonaws.com/shell_server.sql to load initial data into the sql server
  3. then I'll use the seed to seed the sql server

Hope the steps sound about right :)

dennyabrain commented 2 years ago

Yes, they sound right. I will send the .env file on saturday. You wont need to do step 2 any more. Sequelize takes care of initializing the db, tables and the data.you will need to run npx sequelize-cli db:migrate before step 3 to setup the db.

I did have more upto date instructions somewhere. Let me send those too by saturday evening.

dennyabrain commented 2 years ago

@RishavT I have updated the development steps here. You should be able to setup this locally now. Let me know if you face any problem.

https://github.com/tattle-made/kosh-v2/blob/main/docs/dev.md

Related sidenote : I was on a different linux machine right now and was able to use the "copy to clipboard" feature on both chrome and firefox :/ I will keep you posted if I can recreate this issue on my laptop and test your PR later.

RishavT commented 2 years ago

Ah, haha! Sure no worries. I do think a failsafe might be good to have anyway provided you think it's worth the increase in code (navigator.clipboard is null for me on kosh.tattle.co.in, so I'm guessing it might be null for others too). Feel free to merge or scrap at a later time, whatever you feel is appropriate :)

RishavT commented 2 years ago

Interestingly, I can also no longer replicate this issue lol on my local. navigator.clipboard is not null anymore (even in the home page on local). But when I go to kosh.tattke.co.in, navigator.clipboard comes up as null. I'm wondering if this has been fixed by some other push which isn't live yet.

RishavT commented 2 years ago

So I can't replicate the issue - and can't even set navigator.clipboard to null. I guess I'll pause development on this for now @dennyabrain . Let me know if you or anyone faces this issue again on the live website and I can dig deeper :)