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

Download content pack even when there is content #866

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

manuq
Copy link
Collaborator

@manuq manuq commented Oct 3, 2023

Change the assumption for showing the welcome screens: before, when there was content in the database it was assumed to be already downloaded and the download was skipped. Now rely on the presence of a DOWNLOAD_COMPLETED state persisted.

Fix #863

@@ -145,29 +149,26 @@ export function decideWelcome(store) {

return Promise.all([getDownloadStatus(), getShouldResume()]).then(
([status, { shouldResume, grade, name }]) => {
if (_isDownloadOngoing(status) || shouldResume) {
console.log(['MANUQ', status, shouldResume, grade, name]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you drop or replace this string with something more descriptive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Haha, this was a print leftover.

Copy link
Member

@dbnicholson dbnicholson left a comment

Choose a reason for hiding this comment

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

Looks good to me if you can clean up the debug message. One of these days I'll learn how all the routing works... It would also be nice to store the download state in the backend database someday.

Change the assumption for showing the welcome screens: before, when
there was content in the database it was assumed to be already
downloaded and the download was skipped. Now rely on the presence of a
DOWNLOAD_COMPLETED state persisted.

Fix #863
@manuq manuq force-pushed the 863-content-pack-with-channels branch from f21dd08 to a935f95 Compare October 4, 2023 11:29
@manuq
Copy link
Collaborator Author

manuq commented Oct 4, 2023

Looks good to me if you can clean up the debug message. One of these days I'll learn how all the routing works...

The routing is a pain. I have checked in latest Kolibri and the situation didn't improve. So Kolibri has a wrapper around vue-router which plugins use passing a routes object. But also Kolibri has a SET_PAGE_NAME and the current page is used to display a different component in the root view (in our case in the ExplorePlugin). And in the same root has a <router-view />. This is kind of redundant. The Vue Router library should take care of mapping routes to components and display them in the router-view directly, as in the very beggining of their getting started guide: https://router.vuejs.org/guide/

It would also be nice to store the download state in the backend database someday.

Yeah.. that was the idea when implementing content packs management (install more packs, remove packs, etc).

@manuq manuq merged commit 2f6959e into master Oct 4, 2023
@manuq manuq deleted the 863-content-pack-with-channels branch October 4, 2023 12:34
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.

Run content pack selection even if channels exist
3 participants