feat: add navigation links to make footer consistent #13
Conversation
|
This PR should be rebased and then created against indigo/teak, not master. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
src/components/Footer.jsx
Outdated
|
|
||
| <nav className="nav-colophon" aria-label="About"> | ||
| <ol> | ||
| <li><a href={`${config.LMS_BASE_URL}/about`}>About Us</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/blog`}>Blog</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/donate`}>Donate</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/tos`}>Terms of Service</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/privacy`}>Privacy Policy</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/help`}>Help</a></li> | ||
| <li><a href={`${config.LMS_BASE_URL}/contact`}>Contact Us</a></li> | ||
| </ol> | ||
| </nav> |
There was a problem hiding this comment.
This will cause some conflicting behavior across legacy and MFE pages. In tutor-indigo, footer links on legacy pages can be toggled (https://github.com/overhangio/tutor-indigo/blob/release/tutorindigo/templates/indigo/lms/templates/footer.html). In here, the links will show all the times. The behavior needs to be consistent.
src/components/Footer.jsx
Outdated
| } = this.props; | ||
| const showLanguageSelector = supportedLanguages.length > 0 && onLanguageSelected; | ||
| const config = getConfig(); | ||
| const indigoFooterNavLinks = config.INDIGO_FOOTER_NAV_LINK || []; |
There was a problem hiding this comment.
Should there be default links in case the config is not present? Otherwise, it would look like an empty footer.
There was a problem hiding this comment.
@DawoudSheraz We are getting INDIGO_FOOTER_NAV_LINKS variable from indigo side, if there is any default one, will reflect here otherwise it should be same as legacy pages.
There was a problem hiding this comment.
How will it be same as legacy pages?
There was a problem hiding this comment.
because INDIGO_FOOTER_NAV_LINKS is setting from indigo with following command:
tutor config save --set "INDIGO_FOOTER_NAV_LINKS=[{"title": "About", "url": "/about"}, {"title": "Contact", "url": "/contact"}]"
And we are using that same value here in footer component so that the navlinks on both sides will same.
There was a problem hiding this comment.
Let's say you unset the INDIGO_FOOTER_NAV_LINKS variable using the following command:
tutor config save --unset "INDIGO_FOOTER_NAV_LINKS"When this is done, Tutor automatically sets a default set of 7 footer navigation links for all MFEs. So, if INDIGO_FOOTER_NAV_LINKS is not explicitly set, Tutor Indigo provides default navlinks across every MFE.
The following line sets the default value for INDIGO_FOOTER_NAV_LINKS in the Tutor config at this path:
env/apps/openedx/settings/lms/production.py
MFE_CONFIG['INDIGO_FOOTER_NAV_LINKS'] = [
{'title': 'About Us', 'url': '/about'},
{'title': 'Blog', 'url': '/blog'},
{'title': 'Donate', 'url': '/donate'},
{'title': 'Terms of Service', 'url': '/tos'},
{'title': 'Privacy Policy', 'url': '/privacy'},
{'title': 'Help', 'url': '/help'},
{'title': 'Contact Us', 'url': '/contact'}
]To open this file, first run the following command to locate the Tutor environment root:
tutor config printrootThis will output the path to your Tutor project. Navigate to that directory and open the file at:
env/apps/openedx/settings/lms/production.py
Here, you’ll find where Tutor sets the default footer nav links.
However, if we’re not using Indigo (e.g., using this footer component in another theme or in a standalone setup), we default to an empty array, since the default edx-platform configuration does not include any footer navlinks. You can confirm this in the Footer.jsx source code.
DawoudSheraz
left a comment
DawoudSheraz
left a comment
There was a problem hiding this comment.
- Please fix the coverage issue
- Wait for a 2nd review before merge.
arbirali
force-pushed
the
zubair/TIA-137-61
branch
from
afe0392 to
4e78bcc
Compare
|
@zubairshakoorarbisoft I updated the test coverage for the footer nav the test coverage for the footer nav CC: @DawoudSheraz |
|
Can we merge this? If not, what is blocking us? |
|
This PR depends on overhangio/tutor-indigo#167. That PR was closed when teak was merged. @zubairshakoorarbisoft / @arbirali is yet to create a follow-up. |
|
@DawoudSheraz, there is already an open PR related to this overhangio/tutor-indigo#171 |
arbirali
changed the title
|
@ahmed-arb PR title and description changed accordingly. |
7 participants
Description
This PR ensures footer consistency across all Indigo theme pages by using the LMS footer as the default. It also supports additional navigation links via configuration.
Changes
Added support for
INDIGO_FOOTER_NAV_LINKSin config to allow custom navigation links.Fallbacks to
Tutor Indigodefaults when no config is provided.I have created the following PR in indigo, this PR depends on:
overhangio/tutor-indigo#167
Related issue overhangio/tutor-indigo#146
Related Taiga issues:
Before:
After:
