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.

Sign up for GitHub

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

move MeteredConnectionNotificationModal to Library Page & conditionally render "Other Libraries" #11683

Merged
merged 2 commits into from
Jan 5, 2024
Merged

move MeteredConnectionNotificationModal to Library Page & conditionally render "Other Libraries" #11683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45e7a58
move MeteredConnModal from AppBarPage to Learn Library index + cleanu…
thanksameeelian Jan 2, 2024
45d4580
remove extraneous addition to learn module state
thanksameeelian Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions kolibri/core/assets/src/views/CorePage/AppBarPage.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@
@cancel="languageModalShown = false"
/>

<MeteredConnectionNotificationModal />

</div>

</template>
Expand All @@ -69,7 +67,6 @@
import { LearnerDeviceStatus } from 'kolibri.coreVue.vuex.constants';
import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
import { isTouchDevice } from 'kolibri.utils.browserInfo';
import MeteredConnectionNotificationModal from 'kolibri-common/components/MeteredConnectionNotificationModal';
import AppBar from '../AppBar';
import StorageNotification from '../StorageNotification';
import useUserSyncStatus from '../../composables/useUserSyncStatus';
Expand All @@ -78,7 +75,6 @@
name: 'AppBarPage',
components: {
AppBar,
MeteredConnectionNotificationModal,
LanguageSwitcherModal,
ScrollingHeader,
SideNav,
Expand Down
6 changes: 5 additions & 1 deletion kolibri/plugins/learn/assets/src/composables/useDeviceSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { computed } from 'kolibri.lib.vueCompositionApi';
import { computed, ref } from 'kolibri.lib.vueCompositionApi';
import store from 'kolibri.coreVue.vuex.store';
import plugin_data from 'plugin_data';

const allowDownloadOnMeteredConnection = ref(plugin_data.allowDownloadOnMeteredConnection);

export default function useDeviceSettings() {
const allowGuestAccess = computed(() => store.getters.allowGuestAccess);
Expand All @@ -8,5 +11,6 @@ export default function useDeviceSettings() {
return {
allowGuestAccess,
canAccessUnassignedContent,
allowDownloadOnMeteredConnection,
};
}
38 changes: 37 additions & 1 deletion kolibri/plugins/learn/assets/src/views/LibraryPage/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
isOnMyOwnUser
@cancel="hideWelcomeModal"
/>
<MeteredConnectionNotificationModal
v-else-if="usingMeteredConnection"
@update="(value) => allowDownloadOnMeteredConnection = value"
/>
</transition>
<LearnAppBarPage
:appBarTitle="appBarTitle"
Expand Down Expand Up @@ -76,7 +80,7 @@
/>
<!-- Other Libraries -->
<OtherLibraries
v-if="!deviceId && isUserLoggedIn"
v-if="showOtherLibraries"
data-test="other-libraries"
:injectedtr="injecttr"
/>
Expand Down Expand Up @@ -166,12 +170,15 @@
import samePageCheckGenerator from 'kolibri.utils.samePageCheckGenerator';
import { ContentNodeResource } from 'kolibri.resources';
import { mapState, mapGetters } from 'vuex';
import MeteredConnectionNotificationModal from 'kolibri-common/components/MeteredConnectionNotificationModal.vue';
import appCapabilities, { checkCapability } from 'kolibri.utils.appCapabilities';
import SidePanelModal from '../SidePanelModal';
import SearchFiltersPanel from '../SearchFiltersPanel';
import { KolibriStudioId, PageNames } from '../../constants';
import useCardViewStyle from '../../composables/useCardViewStyle';
import useContentLink from '../../composables/useContentLink';
import useCoreLearn from '../../composables/useCoreLearn';
import useDeviceSettings from '../../composables/useDeviceSettings';
import {
currentDeviceData,
setCurrentDevice,
Expand Down Expand Up @@ -204,6 +211,7 @@
ChannelCardGroupGrid,
SidePanelModal,
LearningActivityChip,
MeteredConnectionNotificationModal,
ResumableContentGrid,
SearchResultsGrid,
SearchFiltersPanel,
Expand All @@ -218,6 +226,7 @@
const router = currentInstance.$router;

const { isUserLoggedIn, isCoach, isAdmin, isSuperuser } = useUser();
const { allowDownloadOnMeteredConnection } = useDeviceSettings();
const {
searchTerms,
displayingSearchResults,
Expand Down Expand Up @@ -361,6 +370,7 @@
showLibrary();

return {
allowDownloadOnMeteredConnection,
canAddDownloads,
canDownloadExternally,
displayingSearchResults,
Expand Down Expand Up @@ -402,6 +412,7 @@
isLocalLibraryEmpty: false,
metadataSidePanelContent: null,
mobileSidePanelIsOpen: false,
usingMeteredConnection: true,
};
},
computed: {
Expand All @@ -421,6 +432,19 @@
window.localStorage.getItem(welcomeDismissalKey) !== 'true'
);
},
showOtherLibraries() {
const validUser = !this.deviceId && this.isUserLoggedIn;
if (!validUser) {
return false;
}
if (!checkCapability('check_is_metered')) {
return true;
}
if (this.allowDownloadOnMeteredConnection) {
return true;
}
return !this.usingMeteredConnection;
Copy link
Member

Choose a reason for hiding this comment

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

It took me a bit to understand why !this.usingMeteredConnection was the default return here, even though when I realized, it felt "oh, duh!". The other conditional checks are very easy to follow just by reading through, and I think a very brief comment above would make this easier to understand... even something as basic (and somewhat redundant) as
// other libraries shouldn't display on metered connections unless explicitly permitted
or something along these lines

},
channelsLabel() {
if (this.deviceId) {
if (this.deviceId === this.studioId) {
Expand Down Expand Up @@ -481,6 +505,18 @@
if (window.sessionStorage.getItem(welcomeDismissalKey) !== 'true') {
this.$store.commit('SET_WELCOME_MODAL_VISIBLE', true);
}

// parallels logic for showOtherLibraries
if (
!this.deviceId &&
this.isUserLoggedIn &&
!this.allowDownloadOnMeteredConnection &&
checkCapability('check_is_metered')
) {
appCapabilities.checkIsMetered().then(isMetered => {
this.usingMeteredConnection = isMetered;
});
}
},
methods: {
hideWelcomeModal() {
Expand Down
3 changes: 3 additions & 0 deletions kolibri/plugins/learn/kolibri_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ def plugin_data(self):
from kolibri.core.discovery.well_known import CENTRAL_CONTENT_BASE_INSTANCE_ID

return {
"allowDownloadOnMeteredConnection": get_device_setting(
"allow_download_on_metered_connection"
),
"allowGuestAccess": get_device_setting("allow_guest_access"),
"allowLearnerDownloads": get_device_setting(
"allow_learner_download_resources"
Expand Down
3 changes: 1 addition & 2 deletions packages/kolibri-common/components/MeteredConnectionNotificationModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@
// if we only include one of the keys for the extra_settings object
client({ method: 'GET', url: this.settingsUrl })
.then(({ data }) => {
console.log('mounted', isMetered);
console.log(data);
this.extra_settings = data.extra_settings;
this.selected = this.extra_settings.allow_download_on_metered_connection
? Options.USE_METERED
Expand Down Expand Up @@ -116,6 +114,7 @@
data: { extra_settings },
})
.then(() => {
this.$emit('update', allow_download_on_metered_connection);
window.sessionStorage.setItem(meteredNetworkModalDismissedKey, true);
this.dismissed = true;

Expand Down