Simon-Initiative / oli-torus

Next Generation OLI Authoring and Delivery Platform
https://proton.oli.cmu.edu
MIT License
83 stars 35 forks source link

[ENHANCEMENT] [NG23-178] Improve navigation when prev or next resource is a section or subsection #4857

Closed gastonabella closed 2 months ago

gastonabella commented 3 months ago

NG23-178

Set the following as a lesson navigation rule:

  1. If the next or previous page is a module, redirect the student to the Learn view (/sections/{SECTION_SLUG}/learn), highlighting the container.
  2. Otherwise, redirect to the specified resource view (/sections/{SECTION_SLUG}/lesson/{RESOURCE_SLUG}) (page or unit).
  3. Remove sections and subsections from the footer navigation.

Before:

https://github.com/Simon-Initiative/oli-torus/assets/26532202/a373d684-a4b9-4a0c-ac28-333e7129a995

After:

https://github.com/Simon-Initiative/oli-torus/assets/26532202/11919794-4f30-4956-9b11-6719c93453bf

gastonabella commented 3 months ago

I verified my suspicion by testing this locally: this change now renders "sections" and "sub-sections" as pages, visible to the user. Consider the following course structure:

Unit1
  Module1
     Section1
         page1
         page2
     Section2
         page3

If a student is viewing page2 and clicks "Next", they should next see page3. This impl, however, renders "Section2" as a page.

@darrensiegel I agree that we should not be showing sections and subsections as pages, but this behavior is inherited and caused by the current logic under the Oli.Delivery.PreviousNextIndex (and the :previous_next_index field in Sections). There we are considering both pages and containers as navigable resources when building this structure. There are several options, but I have two at the top of my head:

A. Update how the previous_next_index structure is built to ignore sections and subsections. Since this structure is mainly used for navigation, I don't see the purpose of retrieving these containers. We should rebuild and update this field on all sections too.

B. Keep the current structure but update how it is navigated so we skip sections. Therefore, every time we need to determine what the next/prev resource should be, we iterate over the structure until we find a page or a module/unit.

My only concern is that sections and subsections would lose visibility while navigating through pages. They would only be seen on the Learn page.

Any ideas?

darrensiegel commented 3 months ago

@gastonabella Actually now that I think about it, there is a third option here: Instead of skipping section and sub-sections, we do render them. But we render them not as pages, but as a container. This is how v27 works.

Rendering these resources as a container simply means that we render a table of contents for the pages contained in that container.

Then when a student clicks "Next" again, the will be taken to the first page in that section.

gastonabella commented 3 months ago

@gastonabella Actually now that I think about it, there is a third option here: Instead of skipping section and sub-sections, we do render them. But we render them not as pages, but as a container. This is how v27 works.

Rendering these resources as a container simply means that we render a table of contents for the pages contained in that container.

Then when a student clicks "Next" again, the will be taken to the first page in that section.

@darrensiegel I like that approach! Does that table of contents already exist? Are there any Figma designs? Even though we could do it on this PR, I prefer to be consistent and keep following the good design practices as for NG.

darrensiegel commented 3 months ago

That Table of contents implemetnation still exists in the code. The template that it used is lib/oli_web/templates/page_delivery/container.html.heex

But i'm quite sure that it hasn't been updated for NG designs.

gastonabella commented 3 months ago

@darrensiegel I've just pushed the changes based on the discussion in the Jira ticket comments. The decision was to skip units and sections in the footer navigation and redirect to the Learn page when navigating to a module. This is a temporary workaround to avoid the current behavior. However, I agree that we should aim for a final version where we can navigate to units and sections as well, and build something like a table of contents once we have the designs. I updated the "After" recording so you can see how it works now.

gastonabella commented 2 months ago

@darrensiegel sounds good! I've just pushed the changes.