Many hybrid learning things

[marcellamaki]

Summary

Due to having many open PRs that have been built on top of each other, I have created a new branch that has merged or rebased that work, and then done a polish. This PR covers quite a few front end issues for the hybrid learning work, but hopefully it is in a state where reviewing it together, while extensive, does make testing and QA a bit easier.

There are still a few bugs that are not fixed, but for the most part, the core functionality and mobile responsiveness should be in place. 🤞

References

Updated Lessons Cards on Class Page

Description	Screenshot (minimal interactions to demo in this section, so just the screenshot
Desktop view
Mobile view

Library Page

Description	Gif or Screenshot
"Default"/ Library landing view
Filtering options
Category Search Modal
Mobile Library view and search interactions

Channel Topic Browsing

Description	Gif or Screenshot
Desktop channel browse
Medium screen
Mobile

Bookmarks Page

Bookmarks desktop
Mobile bookmarks, and remove bookmark interaction

Reviewer guidance

There is quite a lot of code in this PR, and I think all of it deserves extra testing. Key points to test for (drawn from other PRs).

Across ALL pages

1. Accurate reflection of Figma specs (cc @jtamiace) - although not all metadata will be displayed

• Updated card designs
• Responsive layout working as expected across browsers and screens
• Have I used the correct cards in all places, and have I interpreted the mobile responsive differences correctly? (I have done by best, but fresh eyes would certainly help because I have been staring at this for a very long time)
• Overall layout
• Buttons and tooltips (note, persistent "active" state not implemented yet)

On Library, and Channel and topics browsing
Filtering and keyword search as implemented by @rtibbles still working 😄. On Library page, filter for all channels that exist on device. On channel page, search results should be limited to the current channel:

• Can search by keyword
• Can search by single filter
• Can search by multiple filters
• Can view more
• Can clear all
• Can clear filters individually
• Side panel accurately disables and/or hides filters and filter groups that are not available on search results

On Bookmarks page

• Complete list of bookmarks displayed
• "Bookmarked time" accurately displayed on card
• Removing a bookmark removes the correct bookmark

General notes and comments

@pcenov has already pointed out that there need to be some improvements made on IE11, and I haven't forgotten this, but I haven't implemented it just yet. Would welcome any feedback (either in the PR, or on Slack or in a call) from @MisRob @nucleogenesis or @indirectlylit about how I can make that experience better.

Another particular area that needs some "look and feel" improvements is the Topics browsing generally... jankiness from the sticky sidebar which I am trying to calculate with computed props. Would love an idea about how to do that differently/correctly/better.

The pages themselves are a bit clunky - I had great aspirations to refactor into some smaller sub components, but for time reasons, I've just tried to add a few comments to layout what is happening overall, and which sections of the file are responsible for what on the Library and Topics pages, which are quite long. If there are any other areas that you think comments would be a helpful addition, please let me know and I will be happy to add before merging - it would be a low lift, and I think quite worth it.

---

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

[MisRob]

Thank you, @marcellamaki

I haven't managed to go through all files yet but am posting what I've noticed so far. I'll continue with the next review batch later. If @nucleogenesis gets to finishing the review sooner than me, feel free to proceed with merging after addressing important feedback.

[MisRob]

@pcenov has already pointed out that there need to be some improvements made on IE11, and I haven't forgotten this, but I haven't implemented it just yet. Would welcome any feedback (either in the PR, or on Slack or in a call) from @MisRob @nucleogenesis or @indirectlylit about how I can make that experience better.

Firstly, can you run IE11 on your machine, in VM for example? Maybe @jacob and @devon will have some general advice, I don't have any. I usually just try to at least briefly preview new components there and if I need to fix something, it usually requires lots of googling for known IE problems. I am glad to help with that, however would need to hear in more detail about particular issues that you have problems with. If it's nothing blocking, we can also open follow-up issues for this.

[pcenov]

Hi @marcellamaki, today I had some time to test mainly the Browse channel pages and here are the findings as compared to Figma:

1. The channel name is not positioned correctly, there is too much white space, the Folders option is not underlined and selected by default, and there is displayed breadcrumb navigation while in Figma it isn't. Also clicking the Channels link doesn't result in anything:

2. Missing 'Show more' option:

3. Resources view - several issues, see the screenshot:

4. End folder view - the Folders section is missing, missing folder title and options

5. Search results: the search tab is not selected and underlined, the cards are squished, the x icon is incorrect, no breadcrumb navigation should be displayed and missing options for grid/list view

Other issues:
7. Both me and Radina saw an issue where the breadcrumb navigation gets mixed up but I didn't have time to find a way to reliably replicate it:

[indirectlylit]

Related to IE11, cross-posting from Slack:

One general rule we used try and enforce in Kolibri was "no flexbox allowed" because IE and early Android implemented different versions of it with slight incompatibilities and gotchas. For example, our KGrid is implemented using inline blocks specifically to avoid flexbox.

Over time my sense is that engineers really wanted to have that tool in our CSS toolbox and we've started using it more liberally. If many of the IE issues we're encountering turn out to be flexbox related, perhaps it's time to revisit this rule and "ban" flexbox again

Here's a recent example from @MisRob's PR

Most of these guidelines aren't relevant to the IE issue, but here's where that "rule" was documented (without justification or explanation unfortunately)

[nucleogenesis]

I think that most of my notes here shouldn't be blocking as long as their tracked in a follow-up issue and handled soon.

The only thing important before release I think is the places where you used this.windowWidth from KResponsiveWindowMixin - there is a stern warning in the mixin not to do that.

The rest are notes about code style/conventions and such for the most part and can be tackled later on in a follow-up issue.

[nucleogenesis]

There are a ton of props here and a lot of them are just extractions of values from content. If you instead just pass one prop :content="content" to the HybridLearningContentCardListView component, then you'll save a lot of code in that component by removing all of the prop definitions and in every component using it as well. The goes for the HybridLearning(Lesson|Content)Card components as well. In those components you can then just reference content wherever you need it's properties.

I don't think that this is blocking as it's mostly stylistic for now - and I'd be happy to write a follow-up issue for this as well.

[marcellamaki]

I actually was wondering about this as I was doing it -- if there were any downsides to just passing the content object and then using computed props (i.e. performance considerations - either good or bad) so thanks for answering that this is a better approach, without me even asking! :)

[rtibbles]

We should definitely do the object based approach - the main caveat, thinking more generally, of passing objects is the lack of specification of what attributes that object should have. Ideally, we should have a companion function to normalizeContentNode - validateContentNode that would assert the presence of all relevant attributes on the content node object, that can then be used for the validator on the prop.

[marcellamaki]

Follow up here #8564

[indirectlylit]

looking great!

[indirectlylit]

more dead code

[rtibbles]

Nothing that I see as blocking - but definitely some things we can follow up on.

[pcenov]

Hi @marcellamaki these are the additional issues I was able to identify after today's round of testing on all the major workflows and both me and Radina don't see as blockers:

1. A long description issue in Browse channel:
   

2. A similar issue in the Bookmarks:
   

3. Search results in mobile view when compared to Figma:
   

4. Sometimes the list/grid toggles appear in mobile view when there is a single column and they shouldn't. I tested this in real mobile phones as well:
   

[marcellamaki]

Follow up for @pcenov's very diligent design alignment, as well as some other styling issues (covering tokens instead of hex, icon usage) here: #8563

[rtibbles]

All comments resolved - manual QA passed. Let's merge this and cross our fingers!