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

Bugfix - copy not working on firefox #64

Closed
wants to merge 2 commits into from
Closed

Conversation

RishavT
Copy link

@RishavT RishavT commented Apr 6, 2022

Fixes #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
Copy link
Contributor

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
Copy link
Contributor

@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 RishavT marked this pull request as draft April 7, 2022 09:10
@RishavT
Copy link
Author

RishavT commented Apr 7, 2022

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
Copy link
Contributor

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

@RishavT
Copy link
Author

RishavT commented Apr 7, 2022

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Author

RishavT commented Apr 22, 2022

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
Copy link
Author

RishavT commented Apr 22, 2022

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
Copy link
Author

RishavT commented Apr 22, 2022

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 :)

@RishavT RishavT closed this Apr 22, 2022
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.

Unable to copy token on Firefox
2 participants