overhangio / tutor-indigo

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

make lms .content-wrapper style rule more specific #23

Closed insad closed 2 years ago

insad commented 2 years ago

The #content.content-wrapper style rule in _extra.scss is not specific enough, without this patch it will also affect course content inside MFE (inside the wrapping iframe), and couse a lot of extra white space and cut-off:

img_1

img_2

This patch only applies the style rules for #content.content-wrapper (in essence pushing this block down in case of a fixed header), if it follows a global header header.global-header, hence will not affect the html inside the MFE iframe.

regisb commented 2 years ago

Hey Albert, thanks for this PR. Can you please post a screenshot of the same page after applying your patch? I'm not sure I understand the problem.

insad commented 2 years ago

Without the patch:

without-patch

With the patch:

with-patch

The problem without the patch is caused here:

affected-div-inside-iframe

Observation: In your demo site https://demo.openedx.overhang.io/ the problem is not visible, because you are not using the fixed header there from the indigo theme (so probably you have the corresponding style rules in _extra.scss commented out). The problem appears with the fixed header of the indigo theme:

home

regisb commented 2 years ago

Observation: In your demo site https://demo.openedx.overhang.io/ the problem is not visible, because you are not using the fixed header there from the indigo theme (so probably you have the corresponding style rules in _extra.scss commented out).

I'm sorry, I don't mean to make things difficult for you but I really don't understand. The indigo theme is deployed on demo.openedx.overhang.io does as-is, without any modification. What should I do to start using the "fixed header"?

insad commented 2 years ago

fixed header

Because of these rules in https://github.com/overhangio/tutor-indigo/blob/master/tutorindigo/templates/indigo/lms/static/sass/partials/lms/theme/_extras.scss

style

But in your demo site, appearently you are not using these rules, as on scrolling your header doesn't stay fixed! No fixed header (your demo) here:

no-fixed-header

regisb commented 2 years ago

@insad you made me realize there is a bug in the indigo plugin. The _extras.scss file is not rendered to the tutor environment. That's because the path to extras.scss contains a "partials" directory and tutor explicitly ignores any file in folders with this name, which is an unfortunate coincidence: https://github.com/overhangio/tutor/blob/669f8363283c5351ff39007e06df42dd1c48b2c7/tutor/env.py#L75

I'll create an issue now and address your PR as soon as I find a solution.

EDIT: here is the issue https://github.com/overhangio/tutor-indigo/issues/24

insad commented 2 years ago

Changed the css style rule and added a comment, like you asked me. Also tested this, works perfectly.