Reopen the last article after switching between menu items

[mossroy]

A cookie might keep the last article

[Jaifroid]

Kiwix-js-windows now implements re-displaying the last article after switching back from "Config", without re-loading the article. (It also disables the link back to the article if the user has switched CSS display mode, as this necessitates reloading the article). I'm in the process of backporting to kiwix-js to commit just the switching between tabs to a new branch built on the main branch (so as to be able to make a PR for testing from main).

The testing branch is here: https://github.com/kiwix/kiwix-js/tree/Return-to-last-article-from-config-without-reloading , but it has no changes yet.

If anyone wants to test the full feature set, they need to do so on kiwix-js-windows until I edit this message.

Edit: backport is now complete - see PR #291 .

[mossroy]

As discussed in #291, I think switching between menus should "naturally" keep the context.
Without doing anything special, the user should see its last article after going to and back from another section. It should only be a matter of not emptying a div, in order to be able to see its content when un-hiding it.
That would be the first step.

A second step would be to store what's necessary in a cookie, so that to be able to re-open the same article after closing/re-opening the app. Which is a bit more complicated because of the different ways to open a ZIM file

[mossroy]

I renamed this issue, to narrow it to what can be implemented for now : be able to return to the same article after going to another section (settings, or about)
As it's not possible to reopen the ZIM file automatically (in a portable and cross-browser way), it's not possible to do more.

[Jaifroid]

OK. It is possible to remember the last page for each ZIM and load it after the user has picked the ZIM. We simply store a cookie value like lastPageLoaded=Page_d'accueil@wikipedia_fr_all_novid_2018-10.zim and load it if the picked ZIM name matches. In Web context, it's not so useful, but in app context it can be very useful (albeit that it has privacy implications).

[mossroy]

You're right

In Web context, it's not so useful, but in app context it can be very useful

That's why I think it's better to keep it in kiwix-js-windows, without backporting it for kiwix-js (where it would only be useful for the dead Firefox OS...).

With this renamed issue, it should be not necessary to store the info in a cookie, and there will be no privacy concern

[Jaifroid]

@D3V-D Issue is partially fixed in that the code to go back to the article view and unhide any article that is currently loaded is in the codebase, but what hasn't been successfully implemented to date is the "unclicking" action on the menu buttons, so you could click on Config, then unclick it and the loaded article would show.

[D3V-D]

@Jaifroid Yeah I realized this a bit belatedly. In the meanwhile, I might be interested in working on this issue, but I'll take a closer look at it first.

[D3V-D]

Okay, so for unclicking, is that whenever I click on the config section the second time? Or is it when I'm on the config section and I click anywhere on the section that isn't a button and it un-highlights the config tab?

I'm a bit unsure as to when or why we would want to go back to the article by "unclicking"

[Jaifroid]

@D3V-D Effectively the "Config" button should remain selected while Config is showing, and then when you click again on it, the button is visually deselected and we return to whatever article is loaded. When a user clicks on "Home", conversely, the landing page will be loaded again. Same with the "About" button (should act same as Config).

You can see, roughly, how this should work by looking at https://pwa.kiwix.org, where that version (with a somewhat different UI) works in this way. To see it properly in action, you need to load a ZIM.

[D3V-D]

@Jaifroid Okay, thanks for the PWA link, I see what you mean now! Clicking on the tab the second time should take you back to the article, and clicking the home tab should reload it. I remember a while back I did the drag-over feature, where when dragging it would open config, but once you drag-exit it went back to the ZIM, I think without fully reloading it (I can check again), maybe that would work just fine here as well? And reloading the ZIM should be easy enough when clicking home

[D3V-D]

Actually I think the PWA may reload the ZIM when unclicking, but I think the Kiwix-JS extension doesn't when drag-exiting.

EDIT: Nevermind, after some testing I don't think either reload the ZIM entirely.

[Jaifroid]

The infrastructure is all there, it's just about hooking the relevant functions into the Config and About buttons, so they unhide the main article when clicked if they are currently selected. And then there's the cosmetic issue of ensuring the buttons look like they are clicked and unclicked.

I don't think the PWA actually reloads the ZIM when clicking on Home, it just calls the goToMainArticle() function just as the current Home button does in Kiwix JS. I don't actually think you need to touch the Home functionality except perhaps to ensure that the Config and About buttons are visually deselected when clicking it. This may already be the case, actually, so I think it's just about hooking up the buttons to the relevant function (I'll check what that function is called in a moment).

[Jaifroid]

@D3V-D It's uiUtil.returnToCurrentPage(). It looks like it deselects the currently pressed buttons already.

[D3V-D]

@Jaifroid Okay, thanks for the info! Hopefully it's as easy as it sounds, but I'll probably work on it sometime next week

[D3V-D]

@Jaifroid Is there some variable that tracks which page we're on (Config, About, or Home)?

[Jaifroid]

While loading an article, you can get currentArticleURL by concatenating dirEntry.namespace + '/' + dirEntry.url. If storing information in localStorage / cookie (using the abstracted settingsStore methods), you would need to add the ZIM name, something like selectedArchive.file.name + '/' + dirEntry.namespace + '/' + dirEntry.url;. You can actually get that from articleContainer.src (the source of the iframe once an article has been loaded into it), but that will have a slightly different format in ServiceWorker mode vs Restricted mode, and we need to support both. So probably safest to construct a string and store it in Storage as lastPageVisit once we're sure we have the correct article.

Note that when I did something similar for the PWA, I had to beware of two issues:

1. It's important to avoid a "crash loop", where the reader attempts to load an article that has some issue that causes the app to crash. When restarted, the app tries to load the same page and crashes again before the user can do anything about it. Therefore, we need a failsafe such as adding lastPageLoad with value OK into Storage. This can be set with a setTimeout some seconds after an article's onload event has fired. If the app has crashed, it won't get set, and then init.js can intervene to erase the lasPageVisit value.
2. For privacy reasons, it is important to have an option to turn this off in Configuration. Imagine one partner in a couple has been searching for "Divorce law" using Kiwix JS with a Wikipedia ZIM. They close the app. Along comes the other partner and opens it, and the app automatically re-opens the "Divorce law" article... 😉

[D3V-D]

@Jaifroid oh that's also good to know, but I meant moreso tracking whether we're currently on config, about, or home tab in the top bar

Edit: Wait,let me actually try something else first

[Jaifroid]

OK, sorry I misunderstood. You can use the methods in https://github.com/kiwix/kiwix-js/blob/main/www/js/lib/uiUtil.js#L760 to determine which section is currently visible. So, basically, just call uiUtil.fromSection() and it will return the name of the currently visible section.

[D3V-D]

@Jaifroid Where can I access the bootstrap styles? E.g the styles for the :focus on the buttons in the top bar. I don't see them in any of the CSS files in the CSS folder.

[Jaifroid]

Bootsrap is loaded here: https://github.com/kiwix/kiwix-js/blob/main/www/index.html#L42 As you can see, it's loaded from the node_modules folder that will only be present on your system if you have done npm install in the root of the repo.

When the bundler (rollup) is run, this file is moved into the app, so it's all self-contained after bundling.

However, that's all a bit moot, because you shouldn't of course modify the source code for bootstrap (this can change whenever a new version is added). You should instead write override styles, if you need to, in app.css. I hope this answers your question!

[D3V-D]

@Jaifroid Ahh, okay. I was wondering since I'd like to remove the focus styles on the buttons and instead apply those styles whenever the page is active. So, how would you say I do this? I assume there's some way I can remove those styles? (I don't really work with bootstrap often sorry)

[Jaifroid]

Hmm. I think :focus is an inbuilt attribute that is added whenever an element has focus, and that's done by the browser. I'm not 100% sure of this, but it rings a bell. This means you can't remove it. But you can override it in app.css (if necessary with !important, though try first without). Then you can add a style for the active class which (I think) is already applied -- oh yes, you can see how active is removed or added, for example, here: https://github.com/kiwix/kiwix-js/blob/main/www/js/lib/uiUtil.js#L942.

[D3V-D]

Yes, you're correct on focus being applied by the browser, but I'm moreso asking if I can somehow just prevent the :focus styles themselves from being implemented at all? :focus can be present without the styling.

Of course, I can just override the styles if need be but I figured it'd probably be better to remove than to override

Edit: I've looked at the link you've provided, that's helpful; I'll probably use the active class myself.

[Jaifroid]

If they're bootstrap-defined styles in bootsrap.min.css, then you can't edit them out. And you've already searched in app.css, so they're not defined there. I think the only solution if you don't like the focus style is to override them. But it's possible that when you write a style for the active class, that will take precedence, so you might not need to override them at all. I'm just guessing now!

[D3V-D]

Got it, time to just test it out on my end, thanks for all the help though I appreciate it

Note: The .active class actually applies to the .li item instead of the actual button, so I may have to make my own class here, but same concept I believe

[D3V-D]

@Jaifroid I've got a working fix, just need to run tests. It's fine in npm run serve, but in npm run preview I'm not seeing the changes. I think I've had this issue before, and I had to go to a different URL or something to see the changes properly; do you know what I'm talking about?

Before I test the other parts I'd like to make sure the preview is working correctly (though the unit tests and e2e tests seem to be passing from what I see, as well as serviceWorker and restricted modes)

[Jaifroid]

Note: The .active class actually applies to the .li item instead of the actual button, so I may have to make my own class here, but same concept I believe

Ah, OK! You might be able to avoid adding another class by using a style rule such as li.active > btn or li.active > .btn if btn is a class, or else li.active .btn if btn isn't a direct child of li...

[Jaifroid]

@Jaifroid I've got a working fix, just need to run tests. It's fine in npm run serve, but in npm run preview I'm not seeing the changes. I think I've had this issue before, and I had to go to a different URL or something to see the changes properly; do you know what I'm talking about?

Before I test the other parts I'd like to make sure the preview is working correctly (though the unit tests and e2e tests seem to be passing from what I see, as well as serviceWorker and restricted modes)

Make sure Bypass AppCache option is turned on in Configuration to bypass caching of old code... It could be that. Or something else. But you could make a PR anyway if it's working with the unbundled version. Sorry, a bit rushed right now!

[D3V-D]

Note: The .active class actually applies to the .li item instead of the actual button, so I may have to make my own class here, but same concept I believe

Ah, OK! You might be able to avoid adding another class by using a style rule such as li.active > btn or li.active > .btn if btn is a class, or else li.active .btn if btn isn't a direct child of li...

Oh, smart. Might have to do that... for now I've got it done with just a new class though, up to you if I should avoid creating a class or not; would probably cut out a lot of JS.

[D3V-D]

@Jaifroid I've got a working fix, just need to run tests. It's fine in npm run serve, but in npm run preview I'm not seeing the changes. I think I've had this issue before, and I had to go to a different URL or something to see the changes properly; do you know what I'm talking about?

Before I test the other parts I'd like to make sure the preview is working correctly (though the unit tests and e2e tests seem to be passing from what I see, as well as serviceWorker and restricted modes)

Make sure Bypass AppCache option is turned on in Configuration to bypass caching of old code... It could be that. Or something else. But you could make a PR anyway if it's working with the unbundled version. Sorry, a bit rushed right now!

No problem! Yeah I had it on, not sure why it doesn't show. I'll try a few more tests soon then I'll just send a PR over with the unbundled version.

[Jaifroid]

No problem, whatever you think is best -- if I see a simpler way in the PR, I can always suggest it.

[D3V-D]

@Jaifroid Not sure if you've already seen it but I've gone ahead and made the PR last night. Let me know if anything looks off