Transition animation cleanup and page animation turned off by default

[Rishabhg71]

closes #1101
closes #78

[Jaifroid]

Should also close #78 and #114.

[Jaifroid]

I was thinking it might be better to separate this into two PRs, one to clean up transition animation, and one to do the library. Generally best not to mix too many issues (even if related) in a PR!

I'm slightly worried there won't be room in the UI for yet another button / menu item, especially after #523 is completed. Why not follow the model of KJSW and have a button in Config that opens a pane in which the library is framed? Or is layout an issue?

[Rishabhg71]

I was thinking it might be better to separate this into two PRs, one to clean up transition animation, and one to do the library. Generally best not to mix too many issues (even if related) in a PR!

Sure, Do you think that slide animation is required we could simply a lot of code by a fade animation

I'm slightly worried there won't be room in the UI for yet another button / menu item, especially after #523 is completed. Why not follow the model of KJSW and have a button in Config that opens a pane in which the library is framed? Or is layout an issue?

Thats one way to fix this issue,do you think we can can add a button on home screen (with welcome text) that will show library?

[Jaifroid]

Sure, Do you think that slide animation is required we could simply a lot of code by a fade animation

Personally, I dislike the slide animation (it makes me feel dizzy!), and I turn it off, but a previous main developer of Kiwix had asked for it and a trainee dev spent some time coding it, so I think we kind of have to maintain it. I think I would like it to be off by default if you agree (that's easy enough to do probably in the params setup in init.js). It also causes problems when it is on because it interrupts actions like jumping to an anchor (see #701), which we have to fix. (You don't have to do that in your PRs unless you see an obvious way to do it, it could be fixed separately.)

Thats one way to fix this issue,do you think we can can add a button on home screen (with welcome text) that will show library?

I'm open to any reasonable solution, and even the separate tab approach, I just don't think we'll have space (take a look at http://pwa.kiwix.org and reduce the screen to mobile-style dimensions) when we fix #523. A link in the Welcome Text that opens the library in a new page of the app (not a new tab of the browser) would be fine, so long as a button or link is also in Configuration.

A previous dev felt it was redundant in this app to provide an in-app library, because we could simply provide a link (as we do) to https://library.kiwix.org, and it doesn't add a huge amount to have the library in-app. But my feeling is that it's disorientating to the average user to be sent out of the app, unless they really choose to do so, and it's better to have an in-app solution even if it's only wrapping an iframe around https://library.kiwix.org. When I coded KJSW (the PWA), library.kiwix.org did not exist, so I wrote a lot of code processing the HTTP responses coming from download.kiwix.org to provide language filters and subject filters, but fortunately, now, all this is done for us by library.kiwix.org, and I don't think we need much if any processing of its output. It also avoids us having to deal with CORS issues.

[Jaifroid]

Thanks for the PR. It looks promising, but I found some glitches. I built a bundled version and ran it in IE11, and this is what I got:

Also the UI Animation option isn't turned off by default when you reset the app. I explain why in a code comment below.

I'll do a code review when you've fixed the glitches and checked it all works smoothly!

[Rishabhg71]

@Jaifroid Do you mind telling me how you are running tests on IE as i dont have IE installed on windows 11 neither i can manage to run it. It seems like you are on lower windows version. I tried running it on IE mode (edge) but that doesnt support dev tools so i cant debug

[Jaifroid]

@RG7279805 You actually can debug IE Mode. Start the app in IE Mode, then do: Windows + R (opens Run dialogue), type in %systemroot%\system32\f12\IEChooser.exe, then run. In IE Chooser, you can choose the debug target.

That might be easier than reactivating IE11. However, if you want to have an icon to open IE11 on your desktop, make a new shortcut, and put this in Target: C:\Windows\System32\mshta.exe javascript:open();close(). "Start in" should be C:\windows\system32. Save it on your Desktop. Hopefully when you double click, it will allow you to open IE11 for testing purposes. Note that IE11 can only run the built app (in dist), and it's best to build it with npm run build-src if you want to debug bundle.js (so you don't get the minified version).

[Jaifroid]

I use IE11 simply as a proxy for the type of issue that might be encountered in older browsers. If it works there, it'll probably work anywhere.

[Rishabhg71]

I tried to recreate this issue but could encounter this glitch at all

Kiwix.-.Profile.1.-.Microsoft.Edge.mp4

[Jaifroid]

OK, I'll see if it also displays in IE Mode for me and get back to you.

[Jaifroid]

I'm afraid I can reproduce this in IE Mode. Please note I have not loaded a ZIM, so this might make a difference. Also, I am not switching fast between tabs, I am simply switching back from About to Configuration, and pause, and the glitch shows.

[Rishabhg71]

Not sure what to do now Should I close the PR and leave it for now? I have tested this multiple times and still don't get this glitch

Kiwix.-.Profile.1.-.Microsoft.Edge.2023-09-12.21-18-06.mp4

[Jaifroid]

@RG7279805 You seem to have compiled a different version of the app, as evinced by the number 3.10.2. You will see that the version I compiled is the one on this branch/PR which is very clearly 3.10.0 (see output of terminal below). Please ensure you are on this branch and are compiling it with npm run build-min. Then test this compilation.

[Rishabhg71]

I am sorry but I think I didn't understand what you were trying to say
I mean on upstream/main aren't we on version 3.10.2?

[Jaifroid]

Yes, but we were not testing upstream/main, you asked me to test this branch. I can only test code you've committed to this branch, and at the time you asked me to review it was on 3.10.0. You've just pushed some updates which will have changed that. My point is only that we need to be testing the exact same code!

[Rishabhg71]

Ok, I finally encountered this issue now on 3.10.2 i can finally try to fix the issue

[Jaifroid]

Yes, I'm still getting the same issue with the code you've just pushed (I hard reset my branch to it, since you force-pushed it). I'm glad you now see it -- hopefully you can debug!

[Rishabhg71]

Sorry for pinging again but could you review again?

[Jaifroid]

Sorry for pinging again but could you review again?

No problem -- please do explicitly ask when you want a review, I had no idea you were waiting for one!

[Jaifroid]

I confirm that the IE11 glitch is now fixed! That's great!

However, I found one slight glitch involving the animation when clicking the "Show article with applied theme" link (after changing from light to dark or vice versa). The animation no longer works with that link (on the main branch, it does work). So the link is no longer interfacing correctly with the new code. Also, the welcome text is erroneously displayed after clicking on that link. Again, this doesn't happen on main. It's easier to show than to explain:

[Rishabhg71]

@Jaifroid Can you take a quick look

[Jaifroid]

Apologies, was caught up in wall-to-wall meetings. Will look no.

[Jaifroid]

Although the erroneous message is fixed if the user has transition animation turned on, it's still there if they're turned off. ☹️

[Jaifroid]

Ah, I see you've just pushed a couple of commits that probably address that!

[Rishabhg71]

Sorry but I am still working on this and didn't know how to take Zim file state so there will be one or two more commits

[Jaifroid]

OK, no problem!

[Rishabhg71]

@Jaifroid could you try once more now it has been fixed

[Jaifroid]

Yes! It now looks like it's fixed. Do you want me to review the code?

[Rishabhg71]

Yes it seems its all good for a review

[Jaifroid]

Great, many thanks for this useful PR. It cleans up a lot of code!
I did a quick test also in IE11, and it seems to be working fine and as intended (after building).
There are some very minor code formatting issues in comments below.
When you've completed everything to your satisfaction, feel free to squash and merge.