overhangio / tutor-indigo

An elegant, customizable theme for Open edX
GNU Affero General Public License v3.0
74 stars 276 forks source link

Unit navigation bar shows empty scroll bar on some screen sizes #10

Closed mrtndwrd closed 3 years ago

mrtndwrd commented 3 years ago

Hey, I have a minimally-adjusted fork of this theme for my project, and I was wondering if you recognize this issue. I can't imagine I caused it, so I can make an upstream PR if necessary:

Here's the issue in our tracker:

https://code.greenhost.net/totem/indigo-totem/-/issues/13

The problem is that when you reduce the width of your screen, at some point a horizontal scroll bar goes over the "unit" navigation bar that's shown above the course content. The negative result of this is that you can't see which unit you are at right now anymore. The link above provides some useful screenshots.

It seems like the problem can be solved by removing the overflow-x: scroll attribute from the object .xmodule_display.xmodule_SequenceModule .sequence-nav .sequence-list-wrapper

regisb commented 3 years ago

Hi @mrtndwrd! I did not manage to reproduce this issue on the demo platform running the latest version of Tutor: https://demo.openedx.overhang.io/courses/course-v1:edX+DemoX+Demo_Course/courseware/interactive_demonstrations/19a30717eff543078a5d94ae9d6c18a5/?activate_block_id=block-v1%3AedX%2BDemoX%2BDemo_Course%2Btype%40sequential%2Bblock%4019a30717eff543078a5d94ae9d6c18a5

On this demo platform, the unit icons are properly resized when the screen is resized. A horizontal scroll bar appears at some point, but it's quite usable, so I can't imagine this being an issue.

resized1 resized2

mrtndwrd commented 3 years ago

Thank you for looking into this! I can actually also reproduce the problem on your demo environment. I think it's important to say that I use Firefox:

image

It happens for me with a screen width between 560 and 976 pixels (smaller than 560 I still see the empty scroll bar, but the highlight is clearer)

regisb commented 3 years ago

The above screenshots were also made on Firefox -- version 85.0 on Linux (Ubuntu). Here's a screenshot made at width=638px for the following url: https://demo.openedx.overhang.io/courses/course-v1:edX+DemoX+Demo_Course/courseware/interactive_demonstrations/19a30717eff543078a5d94ae9d6c18a5/?activate_block_id=block-v1%3AedX%2BDemoX%2BDemo_Course%2Btype%40sequential%2Bblock%4019a30717eff543078a5d94ae9d6c18a5

resized3

There does not seem to be any problem on this screenshot -- right? The only thing that looks slightly off is the horizontal scrollbar at the bottom of the screen.

I observe a similar layout when I log in as a student, both on Firefox and Chromium. On Chromium there is no horizontal scrollbar.

mrtndwrd commented 3 years ago

I'm actually also using Firefox 85 on Ubuntu, and I can reproduce your screenshot.

The problem, apparently, only occurs when you resize the browser window, without using the "responsive design mode" functionality. (It seems like the scroll bars behave differently in responsive design mode :O )

Here's a screenshot of me reproducing the problem in Chromium (Chromium 88.0.4324.96 snap) as well:

image

regisb commented 3 years ago

Right! I finally managed to reproduce your issue. Thanks for bearing with me.

I think the scrollbar is less visible in the mobile emulator simply because its appearance is designed differently. But it's still there.

Indigo does not customize this part of the platform, so it looks like an upstream issue in edx-platform. To answer your question that you asked in a different issue, Indigo overrides the static assets (scss, html, etc.) from edx-platform. So this is where upstream patches should go. On the other hand, Indigo was created precisely to address issues from the default theme, so you should feel free to open a PR right here.

It should be noted that the glitch does not occur on the edx.org website:

resized-edxorg

This is because edX.org does not make use of the default theme, but relies on a custom microfrontend app to display the learning interface: https://github.com/edx/frontend-app-learning

There, the navigation nav is labeled as class="sequence-navigation" and does not have the overflow-x: scroll CSS property that is causing our issue: https://github.com/edx/frontend-app-learning/blob/68c8d31dd15d6bc01f5c70366b82242b35b9f716/src/index.scss#L120

mrtndwrd commented 3 years ago

Fixed upstream, so I'll close this. Thanks for your help!