The hybrid learning home page #8504

MisRob · 2021-10-18T16:59:30Z

Summary

Add a new home page. It doesn't follow the full spec in Figma but is rather an MVP based on #8137. See test scenarios below for details. This PR is meant to be reviewed together with #8430, both code and testing, which contains the majority of work and was merged prematurely due to the string freeze. You're welcome to add review comments to already merged #8430.

References

Reviewer guidance

See the following test scenarios in Notion

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

for the class assignments page. Its subcomponents are already using the composable instead of store. This removes some confusion and fragility around fetching and storing data which was previously done by both composable and Vuex.

To be able to use loading and API error states properly when initalliy loading a page, it needs to respect the standard way pages are initialized in Kolibri - through a route handler.

no matter of their classess memberships, access to unasigned resources settings, or whether they're signed-in or anonymous. From now on, everyone wil lbe taken to the home page which content will adjust correspondingly.

when no channels are available

after navigation in 'Learn'

'Continue learning on your own' and 'Explore channels' should be displayed only after learner finished all their classes resources and quizzes rather than when not having any classes resources or quizzes in progress

MisRob · 2021-10-19T18:07:58Z

I am not sure why the linting check doesn't pass, will investigate later

indirectlylit

Intriguing use of composable as a vuex replacement - I hadn't seen this before. Do you like it better than vuex?

kolibri/plugins/learn/assets/src/composables/useChannels.js

kolibri/plugins/learn/assets/src/views/cards/CardGrid.vue

MisRob · 2021-10-20T08:50:40Z

@jtamiace's feedback:

could we use text.annotation as the color of class names in these cards?

Will update

MisRob · 2021-10-21T13:44:16Z

In "Continue learning on your own", a topic title that should be displayed above a resource title is missing (it's implemented so I might introduce a bug with the most recent updates)

MisRob · 2021-10-21T14:10:28Z

@rtibbles As for API requests that the new home page is responsible for triggering, they're defined in this handler and for example, for a signed-in user (who needs more data than a guest) it's four calls to these API endpoints:

/api/content/channel/?available=true
/learn/api/learnerclassroom/
/api/content/contentnode/<user_id>/resume/
/api/content/contentnodeprogress/

pcenov · 2021-10-21T15:29:21Z

Hi @MisRob, I spent some time yesterday and today testing the buildkite builds on Win10 and Ubuntu, went through all of the neatly written Notion scenarios and I'm happy to report that I didn't uncover any issues with the Home Page or the sections which are displayed depending on various variables such as whether the user is signed in or not and whether the user is enrolled in classes or not. Let me know when you fix the issue with the missing topic title in "Continue learning on your own" and I'll check for regression issues.

MisRob · 2021-10-25T11:23:35Z

Annotation text color is now properly used for a class/topic name on cards

:

MisRob · 2021-10-25T11:24:43Z

Missing topic title in "Continue learning on your own" has been fixed

cc @pcenov

MisRob · 2021-10-25T11:35:33Z

@indirectlylit

Intriguing use of composable as a vuex replacement - I hadn't seen this before. Do you like it better than vuex?

Yes, I'd say it has brought lots of clarity into the home page logic and writing tests even for more complex views that need to use composables feels much simpler than for those utilizing the whole Vuex machinery. Feels like a step towards functional programming with reactivity advantages. I am not sure how it compares to things that need to be handled accross plugins and would like to get a bit more experience - this was the first time I used it in production code since originally I was building it on top of JB's work who started using composables for the home page. If there's anything specific you're curious about, happy to chat more.

MisRob · 2021-10-25T14:34:52Z

While testing something else in IE11, I've noticed two new problems:

Text ellipsis characters showing on cards even when a title is not overflowing:
Broken thumbnails on channel cards: Might be caused by flex being buggy in IE. I used new channel cards with simpler thumbnails styling but until we get to merging our old and new cards components, it may be better to copy the original styling that doesn't seem to break in this way

MisRob · 2021-10-25T15:20:58Z

Tech debt issues related to this PR as well:

when ellipsis should be hidden in IE

MisRob · 2021-10-26T08:44:56Z

While testing something else in IE11, I've noticed two new problems:

Text ellipsis characters showing on cards even when a title is not overflowing

Broken thumbnails on channel cards

Fixed

MisRob · 2021-10-26T14:58:20Z

As far as I can remember, these should be two last tech debt or improvement issues related to the home page:

Add an option to define a default span of all fixed grid items via KFixedGrid prop kolibri-design-system#267
Add "auto" value to KFixedGridItem/KGridItem's "alignment" prop kolibri-design-system#268

MisRob · 2021-10-27T11:21:54Z

@pcenov Thank you for testing. I finished all updates so if you'd like to check it out, I'll appreciate it. I haven't changed any logic related to what sections are displayed, it was rather some smaller UI tweaks that you can read more about in the comments above, so I think it'd be helpful to focus on cards in different resolutions across browsers, especially in IE.

pcenov · 2021-10-27T15:10:00Z

Hi @MisRob all looks very well, I tested in several browsers (including IE11) and different resolutions and was not able to identify any new issues.
Just one question though: there are no time/progress indicators in the 'Continue learning on your own' cards as indicated in Figma. Is this covered in a separate PR?

nucleogenesis

The code looks brilliant. Excellent work with thorough tests, JSDoc comments <3, excellent clarifying comments and good separation of concerns. I enjoyed getting a deeper dive into a and non-trivial real-world composition API implementation!

Do we need these breadcrumbs? Are they in scope here or will they be fixed in follow up?

I'm also seeing things that are off with the card styling where the thing under the ActivityType is centered, which looks odd - see "reference" is unaligned with anything else.

rtibbles · 2021-10-28T21:07:30Z

Do we need these breadcrumbs? Are they in scope here or will they be fixed in follow up?

I think the errant breadcrumbs are fixed in #8548, so we can ignore for now.

rtibbles

Follow up issues filed, and manual testing checks out.

Code read through by @nucleogenesis and a quick skim from me raised no red flags.

MisRob added 17 commits October 18, 2021 18:37

Add tests for recent lessons and quizzes sections

1ef9f6c

Colorful progress icons

340fedf

Show resources progress

f26416d

Truncate cards titles

8f0a1c4

Use useLearnerResources composable

d1e89c6

for the class assignments page. Its subcomponents are already using the composable instead of store. This removes some confusion and fragility around fetching and storing data which was previously done by both composable and Vuex.

Fix loader on the home page and use API error handler

85dfe6c

To be able to use loading and API error states properly when initalliy loading a page, it needs to respect the standard way pages are initialized in Kolibri - through a route handler.

Add docstrings for card grid

d8fbbb4

Add Explore channels section to the home page

8cd9545

More precise tests for routing

8d856e7

Redirect all types of learners to home

a26c045

no matter of their classess memberships, access to unasigned resources settings, or whether they're signed-in or anonymous. From now on, everyone wil lbe taken to the home page which content will adjust correspondingly.

Show resources not available on the home page

c78cd4d

when no channels are available

Do not fetch progress when there are no content nodes

5850b8b

Navigate back to the home page when channel visited from the home page

1769002

Add auto text directionality for user content

7a50ef2

Pass attributes to the inner div

bf853c1

Fix automatic text directionality in KFixedGridItem

91f439a

Make sure that home page is up-to-date

566c971

after navigation in 'Learn'

MisRob force-pushed the home-page branch from 48b1c97 to 566c971 Compare October 19, 2021 08:45

Fix bugs caused by not accepting 0 as a valid resource index

ee1f394

MisRob force-pushed the home-page branch from e383048 to ee1f394 Compare October 19, 2021 11:32

Fix criteria for displaying sections with non-classes content

afc3e36

'Continue learning on your own' and 'Explore channels' should be displayed only after learner finished all their classes resources and quizzes rather than when not having any classes resources or quizzes in progress

MisRob force-pushed the home-page branch from 9ceca35 to afc3e36 Compare October 19, 2021 15:16

MisRob added this to the 0.15.0 milestone Oct 19, 2021

MisRob added the TODO: needs review Waiting for review label Oct 19, 2021

MisRob marked this pull request as ready for review October 19, 2021 17:50

indirectlylit reviewed Oct 19, 2021

View reviewed changes

kolibri/plugins/learn/assets/src/composables/useChannels.js Outdated Show resolved Hide resolved

kolibri/plugins/learn/assets/src/views/cards/CardGrid.vue Show resolved Hide resolved

MisRob added the changelog Important user-facing changes label Oct 21, 2021

MisRob added 5 commits October 25, 2021 11:13

Fix comments

b16ae62

Add a grid type validator

83f51cf

Use text annotation token for collection name color

d16411b

Fix missing quotes in style value

ad3f981

Fix bad property name

c771420

MisRob mentioned this pull request Oct 25, 2021

Experiment with pure CSS alternative to TextTruncator for simpler use cases #8464

Merged

9 tasks

MisRob added 2 commits October 26, 2021 09:50

Fix ellipsis character not being covered fully by white rectangle

996f67d

when ellipsis should be hidden in IE

Fix channel thumbnails images overflowing container in IE

da5e269

Keep proper image ratio

f07d662

MisRob requested review from nucleogenesis, indirectlylit and rtibbles October 26, 2021 15:05

nucleogenesis reviewed Oct 27, 2021

View reviewed changes

nucleogenesis mentioned this pull request Oct 28, 2021

Hybrid Learning Home Page - Follow up issues #8553

Closed

2 tasks

rtibbles approved these changes Oct 28, 2021

View reviewed changes

rtibbles merged commit f642bee into learningequality:release-v0.15.x Oct 28, 2021

MisRob deleted the home-page branch November 4, 2021 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The hybrid learning home page #8504

The hybrid learning home page #8504

MisRob commented Oct 18, 2021 •

edited

Loading

MisRob commented Oct 19, 2021

indirectlylit left a comment

MisRob commented Oct 20, 2021 •

edited

Loading

MisRob commented Oct 21, 2021

MisRob commented Oct 21, 2021

pcenov commented Oct 21, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021 •

edited

Loading

MisRob commented Oct 26, 2021

MisRob commented Oct 26, 2021 •

edited

Loading

MisRob commented Oct 27, 2021

pcenov commented Oct 27, 2021

nucleogenesis left a comment

rtibbles commented Oct 28, 2021

rtibbles left a comment

The hybrid learning home page #8504

The hybrid learning home page #8504

Conversation

MisRob commented Oct 18, 2021 • edited Loading

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

MisRob commented Oct 19, 2021

indirectlylit left a comment

Choose a reason for hiding this comment

MisRob commented Oct 20, 2021 • edited Loading

MisRob commented Oct 21, 2021

MisRob commented Oct 21, 2021

pcenov commented Oct 21, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021

MisRob commented Oct 25, 2021 • edited Loading

MisRob commented Oct 26, 2021

MisRob commented Oct 26, 2021 • edited Loading

MisRob commented Oct 27, 2021

pcenov commented Oct 27, 2021

nucleogenesis left a comment

Choose a reason for hiding this comment

rtibbles commented Oct 28, 2021

rtibbles left a comment

Choose a reason for hiding this comment

MisRob commented Oct 18, 2021 •

edited

Loading

MisRob commented Oct 20, 2021 •

edited

Loading

MisRob commented Oct 25, 2021 •

edited

Loading

MisRob commented Oct 26, 2021 •

edited

Loading