Remove corebase from learn index

[marcellamaki]

Overview

Fixes #9137

This PR removes CoreBase from LearnIndex.vue and adopts the <router-view/> pattern created by @nucleogenesis in his FacilityIndex refactor. (For more background on the logic of the architecture, see #9196)

There are two wrapper pages, LearnAppBarPage and ImmersivePageRoot that manage the style of the page.

In the Learn plugin versions, I've added a new prop applyStandardLayout which defaults to true and applies the default styles of each type of page, but can also be overridden, such as in the Library and Topics pages, where the use the full screen, rather than a centered block of main content.

This PR also removes LearnImmersiveLayout from LearnIndex and puts it in a router-view (although this is probably out of scope) for the sake of consistency. I have not addressed the rather chaotic backRoute situation in LearnImmersiveLayout, but the existing logic has been moved out of LearnIndex and is working.

Places of consideration

Basically all of Learn has been refactored, so this will require a thorough going-over with manual QA. I have tried to be very thorough with my testing on multiple browsers and screen sizes in advance of this PR, but... definitely probably that I have missed something. On the flip side, very few of the code changes are different from the pattern @nucleogenesis set up, and they actually are mostly straightforward (I hope).

Tricky spots were mostly around content since it is no longer passed from LearnIndex. So, things to think about here:

1. Is actually working everywhere?
2. Are there any places where I dropped the loading state handling and things look janky while content is being fetched?

CSS changes to note:

1. (especially @MisRob) I want to be sure the improvements made for 0.15.1 fixes, especially around the scrolling header jumpiness and the height of various elements of the page layout has not been lost. It looks okay to me, and I have done QA on multiple browsers, but your attention and review there would be helpful in case I accidentally removed something that was needed. (cc @pcenov and @radinamatic for testing on IE)
2. I have had to add a z-index to the ImmersiveToolbar that is in LearnIndex due to the cards (without it, they scroll over it). Is this okay? Is there something I should do instead that would make adding an explicit z-index unnecessary?
3. I updated the card width property which resolved the problems with the grid. I do not know why this is necessary in this version 🙃 but it seems to be working now. Are there any places on Library or Topics page where the grid layout is wrong?

[github-actions]

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0.dev0_git.20220609135753.pex
Unsigned Windows installer	kolibri-0.16.0.dev0+git.20220609135753-unsigned.exe
Debian Package	kolibri_0.16.0.dev0+git.20220609135753-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0.dev0+git.20220609135753-0.3.0.dmg
Source Tarball	kolibri-0.16.0.dev0+git.20220609135753.tar.gz
WHL file	kolibri-0.16.0.dev0+git.20220609135753-py2.py3-none-any.whl

[nucleogenesis]

Overall, this looks great! I'm really happy that you found it relatively easy to take the patterns over. I've left a few comments requesting changes:

• One cleaning up a function a bit
• Open for discussion re: the styles overriding, just want to hear your and @MisRob 's thoughts about my suggestion vs keeping as-is
• Optional and non-blocking suggestion that maybe we move ImmersivePageRoot to core (and clean up Facility's version and imports in the process - but can do that in follow-up too)

[nucleogenesis]

I think this might be worth having as a Core component - I think I probably should have done this in the Facility PR as I had no good reason not to - also your version addresses the lack of KLinearLoader in the Facility ImmersiveToolbar which we'll need in that plugin too.

If that's not too much to add into this PR, then you could delete the one in Facility and update the imports (and maybe add it to the API Spec).

[nucleogenesis]

Otherwise we can make a follow-up issue to do that before moving on into the other components for CoreBase extraction

[MisRob]

Good work, thank you @marcellamaki. Overall, it's looking really great.

Regarding code, I left several comments, most of them non-blocking. I think it's important to resolve loading v-ifs in slots and also need some clarification in regard to linear loaders.

I noticed some issues when testing sidebar scrolling locally and maybe one or two other problems. I want to check those places on develop at first to be sure it's not broken there. I'll report it all in another comment later.

[MisRob]

I'd suggest not using _Page in layout components names to make their purpose clear and to allow ourselves easily differentiate between layout components and real pages connected to routes. Maybe LearnAppBarLayout?

[MisRob]

@marcellamaki I tested only briefly and in Chrome. So far I've noticed the following problems that don't seem to be present on develop:

(1) Blank page after navigating to a channel for the first time from the library or after the channel page reload

1. Go to the "Library" and open a channel folder in the "Channels" section (make sure that you haven't visited the channel folder before, for example by reloading the "Library" page)
2. See the blank page and "TypeError: Cannot read properties of undefined (reading 'modality')" error in the console

Interestingly, when I navigate back to the library from the blank page and click again the channel folder, I can then see the channel as expected. However, reloading it results in the same blank page and console error.

(2) Scrolling behavior

• There are two vertical scrollbars when browsing a channel in "Library"
• There seems to be a regression with the stickiness of the top bar when I scroll up and down a couple of times:

Both "China" and "Russia" items in the side panel should be visible. Note that the problem with "China" disappearing is present on develop (might be a regression with calculating the height of the top bar I'd guess), however, I don't see a problem with stickiness not working at all after a while on develop.

[marcellamaki]

Thanks @MisRob and @nucleogenesis for your comments here! Sorry it has taken me so long to get back to them. I hope to have the changes ready for review by end of day tomorrow.

[rtibbles]

As we're doing a refactor, I'm going to be slightly picky and request changes rather than just comment for these things.

All comments addressed.

[MisRob]

@marcellamaki Thank you for all the new work on this. Did you have a chance to look into the problem with the blank page and scrolling behavior, too (#9222 (comment))? Should I re-review something or is it fine to just dismiss my review? Wouldn't like to block this and can see that other folks had a chance to review while I was gone. Please let me know if I can do something to support. Thank you.

[marcellamaki]

Hi @MisRob -- I remember looking into them, but now I cannot remember if I made and committed the changes. I will review and follow up with you if a re-review will be helpful! Thank you

[nucleogenesis]

@marcellamaki - #9335 is merged so this should be hopefully unblocked except for by the conflicts that I've created 😅

[marcellamaki]

I believe all feedback has now been addressed, and this is ready for a final review (and hopefully approval, or close to it! 🤞)

[radinamatic]

@pcenov Let's test this tomorrow, thank you!

feedback addressed