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

Add window reload to startOver() function #25

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kohrVid
Copy link
Contributor

@kohrVid kohrVid commented Jan 21, 2024

This should fix #22, removing the need for the repeated calls to generateImage() when profile pictures are downloaded.

src/app/page.tsx Outdated
Comment on lines 67 to 78
// TODO: Fix if possible. This is a hack to ensure that image generated is as expected. Without repeating generateImage(), at times, the image wont be generated correctly.
await generateImage()
await generateImage()
await generateImage()
Copy link
Contributor

Choose a reason for hiding this comment

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

were you able to test this on a mobile device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't tested this on a mobile device unfortunately. It works when I use the mobile simulator in DevTools on Chrome (and Chromium). Is that ok? If not, are there other ways that you normally test the PRs in this repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me test it on the phone tonight

they way I would test is that after running the app on my local machine, I can then connect my phone to the same network as my machine, and use the local IP assigned by my router to my machine in the phone, i.e. my current local IP is:
Screenshot 2024-01-23 at 12 38 34 PM

I will connect to the same network and visit http://192.168.18.15:3000 then I will be able to visit this site on my phone

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry the issue still exists on the mobile phone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's weird. I just set up ngrok and tried testing this branch in Chrome for Android as well as Firefox Focus and Brave: it was a little slow to load for me but I was able to save the different profile pictures just fine. Are you using an iPhone by any chance?

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is not with that, that PR of yours was I believe merged last week, where a user couldn't re use a different image.

This one relates to multiple await to download an image

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, they have merged the wrong one

#22

Keep the multiple awaits, we can fix it in another PR, just have the windows.reload feature to start over

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Just to confirm, you want me to drop the last commit with the query string (84b2e08) and alter the first commit (03379fc) so that it only contains the window.reload (no fixes to the multiple awaits), right?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, let's create a new PR for resolving the multiple reloads

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just pushed a version of the commit with the multiple awaits re-added.

Twitter and Github profiles work but now all of a sudden I can't get Gitlab profile pictures to download. Seems to be a CORS issue. It's very strange because I was sure it worked last week and I don't think any of the changes made recently would explain this. Does it still work for you? If not, I might need to raise another issue 😕

This removes the need for the repeated calls to `generateImage()` when
profile pictures are downloaded.
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