shinokada / svelte-utterances

svelte-utterances.vercel.app
12 stars 1 forks source link

UI bug fix (2️⃣) #10

Closed amr3k closed 1 year ago

amr3k commented 2 years ago

2nd attempt because my first attempt caused a unwanted chages

This is a fix for #4

The iframe was not applying theme on load until the user switches theme. Now I added an event listener that post a message to iframe window to set the desired theme.

A live demo showing this exists in my blog.

Hint

  1. Switch to dark theme in footer
  2. Refresh the page or navigate to any other page
  3. Return to any blog post and scroll down to comments.
  4. See if the dark theme is automatically applied.

Please hit Ctrl+Shift+R when you visit the second link to clear browser cache.

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/okadashinichi/svelte-utterances/D8BsxgHcCr4r7PjN2dXwaQdPE2tr
✅ Preview: https://svelte-utterances-git-fork-416d72-main-okadashinichi.vercel.app

shinokada commented 2 years ago

Thanks for the PR. I can't see why we need the sendPostMessage function. It works without it as well. Can you explain the reason behind? Thanks.

amr3k commented 2 years ago

@shinokada , sure, the sendPostMessage function is called after the component is mounted. This is a fix for #4 where if you set the theme to dark mode, and navigated to another page then returned back, you'll see utterances went back to light mode despite the whole site is in dark.

The solution I made is to call that function after mount, and also check for theme changes with reactivity so users would be able to dynamically switch utterances theme with the website theme.

This function has to exist instead of writing postMessage inline, to be able to use recursion as iFrame may take some time until it loads.

Please take a look at this:

A live demo showing this exists in my blog.

Hint

  1. Switch to dark theme in footer
  2. Refresh the page or navigate to any other page
  3. Return to any blog post and scroll down to comments.
  4. See if the dark theme is automatically applied.

Please hit Ctrl+Shift+R when you visit the second link to clear browser cache.

shinokada commented 2 years ago

Thanks. I can see it in action on your website. It looks good. Is it possible to add a simple example in the src/routes directory to show how it works? With my current demo site setting, I don't think it works.

amr3k commented 2 years ago

Thanks. I can see it in action on your website. It looks good. Is it possible to add a simple example in the src/routes directory to show how it works?

No problem, this would have to be a basic sveltekit-based routing and a theme store that utilizes localStorage for persistence, I can do it tomorrow (It's 3 A.M here 🙃)

With my current demo site setting, I don't think it works.

Because you are not using a store, so theme changes don't persist.

shinokada commented 2 years ago

If you can add a short explanation, it would be very helpful for users.

amr3k commented 2 years ago

With my current demo site setting, I don't think it works.

I just looked into the demo website once more, and I was wrong, the reason is because you are using an old version of this package, which doesn't include the fix I made in this PR aka sendPostMessage function. If you accepted this PR and merged it with demo-site branch, you'll see the new version running.

shinokada commented 2 years ago

I have done it locally and it doesn't work with my demo-site setup.

amr3k commented 2 years ago

Are you using a store & sync it with localStorage?

Also are you using Firefox? Because I saw some issue with it that didn't happen in Chrome

amr3k commented 2 years ago

This is working proof of my modifications

Please take a look at the newest preview

Set the theme to dark and refresh the page

shinokada commented 2 years ago

Tested on FF and Brave(Chrome).

  1. First load image

  2. Click the darkmode image

  3. Refreshed. It keeps the dark mode but Utterances goes back to light theme. image

And on Brave(Chrome) behaves strange toggling the dark mode. The Utterances theme is opposite of the site theme.

amr3k commented 2 years ago

This is weird, I'm going to do more investigation

amr3k commented 2 years ago

Just looked it up again and fixed that bug.

Let me know if it works.

shinokada commented 2 years ago

Thank you for the PR. I will look into it shortly.

amr3k commented 2 years ago

@shinokada Please review this PR

shinokada commented 2 years ago

Sorry for the delay. This is what you have, demo.

Is it possible to add dark mode as you did it before?

Currently when I refresh the browser, the dark mode is gone.

It will be nice to have a practical example.