Skip to content

Remote Content Browsing: Clean the library page when displaying other available libraries #10216

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

Closed
AllanOXDi wants to merge 13 commits into from
Closed

Remote Content Browsing: Clean the library page when displaying other available libraries #10216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c4f6751
fixes device icon
AllanOXDi Mar 9, 2023
8db2f32
fixes device router
AllanOXDi Mar 9, 2023
73f4ca3
fixed the number of channels not rendering on the cards
AllanOXDi Mar 10, 2023
fee35c9
fixes pinned devices channels
AllanOXDi Mar 15, 2023
9d0a53f
removed hard coded pinned devices
AllanOXDi Mar 16, 2023
8c80ef3
fixed merge conflict
AllanOXDi Mar 16, 2023
fe83049
made the alldevices prop required
AllanOXDi Mar 21, 2023
d30b27f
removed unncessary check on devices
AllanOXDi Mar 21, 2023
a2a9d5d
implemented the dry principal
AllanOXDi Mar 21, 2023
4b9edf3
Merge branch 'develop' into remote_content_browsing_functionality
AllanOXDi Mar 21, 2023
f18797d
fixed failing test
AllanOXDi Mar 21, 2023
c62027b
Used props on the morenetworkdevices component
AllanOXDi Mar 24, 2023
93c1bd2
fixes chanells not displaying corectly
AllanOXDi Mar 29, 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
11 changes: 10 additions & 1 deletion kolibri/plugins/learn/assets/src/views/ChannelCard.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,18 @@
:value="numCoachContents"
:isTopic="true"
/>

<div
v-if="version"
class="version-wrapper"
:style="versionStyle"
>
<p>{{ $tr('version', { version: version }) }}</p>
</div>
<div
style="text-align:right;margin-right:10px;margin-top:10px"
>
<KIcon v-if="isPinned" icon="wifi" />
Copy link
Member

@akolson akolson Mar 30, 2023

Choose a reason for hiding this comment

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

I think the v-if="isPinned" directive should be moved to the parent(div) as hiding just the KIcon will leave additional margins that may not be desirable. Also, see my comments below regarding the isPinned prop

</div>

</router-link>

Expand Down Expand Up @@ -113,6 +117,11 @@
required: false,
default: null,
},
isPinned: {
Copy link
Member

@akolson akolson Mar 30, 2023

Choose a reason for hiding this comment

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

I think this prop is misleading. I think the wifi icon is meant to specify whether the resource is available or not. @jtamiace , @marcellamaki ?

type: Boolean,
required: true,
default: false,
},
},
computed: {
...mapGetters(['isLearner', 'isUserLoggedIn']),
Expand Down
15 changes: 3 additions & 12 deletions kolibri/plugins/learn/assets/src/views/LibraryPage/CardContent.vue
View file
Open in desktop
Copy link
Member

@akolson akolson Mar 30, 2023

Choose a reason for hiding this comment

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

This file should be deleted as its no longer used in light of the discussion here

Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@
class="card-main-wrapper"
:style="cardStyle"
>
<div class="" style="width:250px;height:250px;text-align:center;margin-top:80px">
<h2>
{{ body }}
</h2>
</div>


</div>

</template>
Expand All @@ -28,13 +25,7 @@
windowGutter,
};
},
props: {
body: {
type: String,
required: false,
default: null,
},
},
props: {},

computed: {
cardStyle() {
Expand Down
32 changes: 10 additions & 22 deletions kolibri/plugins/learn/assets/src/views/LibraryPage/MoreNetworkDevices.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
>
<UnPinnedDevices
:deviceName="device.device_name"
:channels="device.channels"
:channels="device.channels.length"
:allDevices="device"
:operatingSystem="device.operatingSystem"
/>
</KGridItem>
</KGrid>
Expand All @@ -19,8 +21,6 @@
<script>

import useKResponsiveWindow from 'kolibri.coreVue.composables.useKResponsiveWindow';
import useDevices from '../../composables/useDevices';
import useChannels from '../../composables/useChannels';
import UnPinnedDevices from './UnPinnedDevices';

export default {
Expand All @@ -30,19 +30,18 @@
},
setup() {
const { windowBreakpoint } = useKResponsiveWindow();
const { fetchChannels } = useChannels();
const { baseurl, fetchDevices } = useDevices();
return {
baseurl,
fetchDevices,
fetchChannels,
windowBreakpoint,
};
},
props: {
devices: {
type: Array,
required: true,
},
},
data() {
return {
devices: [],
};
return {};
},
computed: {
cardColumnSpan() {
Expand All @@ -52,17 +51,6 @@
return 3;
},
},
created() {
this.fetchDevices().then(devices => {
const device = devices.filter(d => d.available);
device.forEach(element => {
this.fetchChannels({ baseurl: element.baseurl }).then(channel => {
element['channels'] = channel.length;
});
this.devices.push(element);
});
});
},
};

</script>
Expand Down
94 changes: 34 additions & 60 deletions kolibri/plugins/learn/assets/src/views/LibraryPage/PinnedNetworkResources.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,23 @@
<template>

<KGrid>
<KGrid v-if="pinnedDevices !== null">
<KGridItem
v-for="group in groups"
:key="group.id"
v-for="device in pinnedDevices"
:key="device.id"
>
<KGridItem>
<h2>
<KIcon icon="device" />
<span class="device-name">{{ group.deviceName }}</span>
<KIcon :icon="getDeviceIcon(device)" />
<span class="device-name">{{ device.device_name }}</span>
</h2>
</KGridItem>
<KGridItem
v-for="item in group.content"
:key="item.id"
:layout="{ span: cardColumnSpan, alignment: 'auto' }"
>
<CardContent
:content="item.title"
:body="item.body"
/>
</KGridItem>

<ChannelCardGroupGrid
data-test="channel-cards"
class="grid"
:contents="device.channels"
:isPinned="true"
/>
</KGridItem>
</KGrid>

Expand All @@ -30,66 +27,43 @@
<script>

import useKResponsiveWindow from 'kolibri.coreVue.composables.useKResponsiveWindow';
import CardContent from './CardContent';
import useDevices from './../../composables/useDevices';
import ChannelCardGroupGrid from './../ChannelCardGroupGrid';

export default {
name: 'PinnedNetworkResources',
components: {
CardContent,
ChannelCardGroupGrid,
},
setup() {
const { windowBreakpoint } = useKResponsiveWindow();
const { baseurl, fetchDevices } = useDevices();
Copy link
Member

@akolson akolson Mar 21, 2023

Choose a reason for hiding this comment

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

Please see my general comment on the LibraryPage/index.,vue file

return {
windowBreakpoint,
fetchDevices,
baseurl,
};
},
props: {
pinnedDevices: {
type: Array,
required: true,
},
},
data() {
return {
groups: [
{
id: 1,
deviceName: 'Samson`s MacBook-Pro',
content: [
{
id: 1,
title: 'Card 1',
body: ' ',
},
{
id: 2,
title: 'Card 2',
body: 'Explore',
},
],
},
{
id: 2,
deviceName: 'Marcella MBP',
content: [
{
id: 1,
title: 'Card 1',
body: ' ',
},
{
id: 2,
title: 'Card 2',
body: ' ',
},
],
},
],
};
return {};
},
computed: {
cardColumnSpan() {
if (this.windowBreakpoint <= 2) return 4;
if (this.windowBreakpoint <= 3) return 6;
if (this.windowBreakpoint <= 6) return 4;
return 3;
methods: {
getDeviceIcon(device) {
if (device['operating_system'] === 'Android') {
return 'device';
} else if (!device['subset_of_users_device']) {
return 'cloud';
} else {
return 'laptop';
}
},
},
created() {},
};

</script>
Expand Down
45 changes: 38 additions & 7 deletions kolibri/plugins/learn/assets/src/views/LibraryPage/UnPinnedDevices.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@
class="card-main-wrapper"
:style="cardStyle"
>
<div class="" style="width:250px;height:100px;">

<KRouterLink
v-if="allDevices !== null"
:text="allDevices.nickname.length ? allDevices.nickname : allDevices.device_name"
:to="{ name: 'LIBRARY', params: { deviceId: allDevices.id } }"
style="text-decoration:none;"
>
<h2 class="device-name">
<span>
<KIcon icon="device" />
<KIcon :icon="getDeviceIcon()" />
</span>
<span>
<TextTruncator
Expand All @@ -16,10 +22,11 @@
/>
</span>
</h2>
<p class="channels">
{{ $tr('channels', { count: channels }) }}

<p v-if="channels" class="channels">
{{ channels }} {{ $tr('channels', { count: channels }) }}
</p>
</div>
</KRouterLink>
</div>

</template>
Expand Down Expand Up @@ -50,8 +57,16 @@
},
channels: {
type: Number,
required: true,
},
allDevices: {
type: Object,
required: true,
},
operatingSystem: {
type: String,
required: false,
default: 0,
default: null,
},
},

Expand All @@ -62,12 +77,26 @@
color: this.$themeTokens.text,
marginBottom: `${this.windowGutter}px`,
minHeight: `${this.overallHeight}px`,
textAlign: 'center',
};
},
},
mounted() {},

methods: {
getDeviceIcon() {
if (this.operatingSystem === 'Android') {
return 'device';
} else if (!this.allDevices.subset_of_users_device) {
return 'cloud';
} else {
return 'laptop';
}
},
},
Copy link
Member

@marcellamaki marcellamaki Mar 28, 2023

Choose a reason for hiding this comment

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

I had to add another } here when running locally.

$trs: {
channels: {
message: '{count, plural, one {channel} other {channels}',
message: '{count, plural, one {channel} other {channels} }',
context: 'Indicates the number of channels',
},
},
Expand Down Expand Up @@ -111,6 +140,8 @@
}

.channels {
justify-content: center;
width: 100%;
color: #616161;
text-align: center;
}
Expand Down
Loading