From db3fb533ba1d15a4dea832b07bed9f874e4cbcc8 Mon Sep 17 00:00:00 2001 From: Diana Olarte Date: Fri, 26 Jan 2024 20:16:22 +1100 Subject: [PATCH 1/5] feat: add nau basic customization (#1) * feat: add nau basic customization * fix: remove unnecessary courseOrg as argument --- src/learning-header/LearningHeader.jsx | 36 +++++++++---- src/learning-header/LearningHeader.test.jsx | 16 ++++-- src/learning-header/data/api.js | 22 ++++++++ src/learning-header/data/api.test.js | 59 +++++++++++++++++++++ 4 files changed, 118 insertions(+), 15 deletions(-) create mode 100644 src/learning-header/data/api.js create mode 100644 src/learning-header/data/api.test.js diff --git a/src/learning-header/LearningHeader.jsx b/src/learning-header/LearningHeader.jsx index 373001d19..feae1cfab 100644 --- a/src/learning-header/LearningHeader.jsx +++ b/src/learning-header/LearningHeader.jsx @@ -1,4 +1,4 @@ -import React, { useContext } from 'react'; +import React, { useContext, useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { getConfig } from '@edx/frontend-platform'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; @@ -7,6 +7,7 @@ import { AppContext } from '@edx/frontend-platform/react'; import AnonymousUserMenu from './AnonymousUserMenu'; import AuthenticatedUserDropdown from './AuthenticatedUserDropdown'; import messages from './messages'; +import getCourseLogoOrg from './data/api'; const LinkedLogo = ({ href, @@ -26,9 +27,16 @@ LinkedLogo.propTypes = { }; const LearningHeader = ({ - courseOrg, courseNumber, courseTitle, intl, showUserDropdown, + courseOrg, courseTitle, intl, showUserDropdown, }) => { const { authenticatedUser } = useContext(AppContext); + const [logoOrg, setLogoOrg] = useState(null); + + useEffect(() => { + if (courseOrg) { + getCourseLogoOrg().then((logoOrgUrl) => { setLogoOrg(logoOrgUrl); }); + } + }, []); const headerLogo = ( {intl.formatMessage(messages.skipNavLink)}
{headerLogo} -
- {courseOrg} {courseNumber} - {courseTitle} +
+ { + (courseOrg && logoOrg) + && {`${courseOrg} + } + + {courseTitle} +
{showUserDropdown && authenticatedUser && ( - + )} {showUserDropdown && !authenticatedUser && ( - + )}
@@ -63,7 +79,6 @@ const LearningHeader = ({ LearningHeader.propTypes = { courseOrg: PropTypes.string, - courseNumber: PropTypes.string, courseTitle: PropTypes.string, intl: intlShape.isRequired, showUserDropdown: PropTypes.bool, @@ -71,7 +86,6 @@ LearningHeader.propTypes = { LearningHeader.defaultProps = { courseOrg: null, - courseNumber: null, courseTitle: null, showUserDropdown: true, }; diff --git a/src/learning-header/LearningHeader.test.jsx b/src/learning-header/LearningHeader.test.jsx index 3d80888ef..d25a34d2a 100644 --- a/src/learning-header/LearningHeader.test.jsx +++ b/src/learning-header/LearningHeader.test.jsx @@ -1,9 +1,13 @@ import React from 'react'; import { - authenticatedUser, initializeMockApp, render, screen, + authenticatedUser, initializeMockApp, render, screen, waitFor, } from '../setupTest'; import { LearningHeader as Header } from '../index'; +jest.mock('./data/api', () => ({ + getCourseLogoOrg: jest.fn().mockResolvedValue(Promise.resolve('logo-url')), +})); + describe('Header', () => { beforeAll(async () => { // We need to mock AuthService to implicitly use `getAuthenticatedUser` within `AppContext.Provider`. @@ -18,12 +22,16 @@ describe('Header', () => { it('displays course data', () => { const courseData = { courseOrg: 'course-org', - courseNumber: 'course-number', courseTitle: 'course-title', }; render(
); + waitFor( + () => { - expect(screen.getByText(`${courseData.courseOrg} ${courseData.courseNumber}`)).toBeInTheDocument(); - expect(screen.getByText(courseData.courseTitle)).toBeInTheDocument(); + expect(screen.getByAltText(`${courseData.courseOrg} logo`)).toHaveAttribute('src', 'logo-url'); + expect(screen.getByText(`${courseData.courseOrg}`)).toBeInTheDocument(); + expect(screen.getByText(courseData.courseTitle)).toBeInTheDocument(); + }, + ); }); }); diff --git a/src/learning-header/data/api.js b/src/learning-header/data/api.js new file mode 100644 index 000000000..41adec453 --- /dev/null +++ b/src/learning-header/data/api.js @@ -0,0 +1,22 @@ +import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; + +const getCourseLogoOrg = async () => { + try { + const orgId = window.location.pathname.match(/course-(.*?):([^+]+)/)[2]; + const { data } = await getAuthenticatedHttpClient() + .get( + `${getConfig().LMS_BASE_URL}/api/organizations/v0/organizations/${orgId}/`, + { useCache: true }, + ); + return data.logo; + } catch (error) { + const { httpErrorStatus } = error && error.customAttributes; + if (httpErrorStatus === 404) { + return null; + } + throw error; + } +}; + +export default getCourseLogoOrg; diff --git a/src/learning-header/data/api.test.js b/src/learning-header/data/api.test.js new file mode 100644 index 000000000..9bf629ca6 --- /dev/null +++ b/src/learning-header/data/api.test.js @@ -0,0 +1,59 @@ +import { getConfig } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; +import getCourseLogoOrg from './api'; +import { initializeMockApp } from '../../setupTest'; + +jest.mock('@edx/frontend-platform/auth'); + +class CustomError extends Error { + constructor(httpErrorStatus) { + super(); + this.customAttributes = { + httpErrorStatus, + }; + } +} + +describe('getCourseLogoOrg', () => { + beforeEach(async () => { + // We need to mock AuthService to implicitly use `getAuthenticatedHttpClient` within `AppContext.Provider`. + await initializeMockApp(); + delete window.location; + getAuthenticatedHttpClient.mockReset(); + }); + + it('should return the organization logo when the URL is valid', async () => { + window.location = new URL(`${getConfig().BASE_URL}/learning/course/course-v1:edX+DemoX+Demo_Course/home`); + getAuthenticatedHttpClient.mockReturnValue({ + get: async () => Promise.resolve({ + data: { + logo: 'https://example.com/logo.svg', + }, + }), + }); + const logoOrg = await getCourseLogoOrg(); + expect(logoOrg).toBe('https://example.com/logo.svg'); + }); + + it('should return null when the organization logo is not found', async () => { + window.location = new URL(`${getConfig().BASE_URL}/learning/course/course-v1:edX+DemoX+Nonexistent_Course/home`); + getAuthenticatedHttpClient.mockReturnValue({ + get: async () => { + throw new CustomError(404); + }, + }); + const logoOrg = await getCourseLogoOrg(); + expect(logoOrg).toBeNull(); + }); + + it('should throw an error when an unexpected error occurs', async () => { + const customError = new CustomError(500); + window.location = new URL(`${getConfig().BASE_URL}/learning/course/course-v1:edX+DemoX+Demo_Course/home`); + getAuthenticatedHttpClient.mockReturnValue({ + get: async () => { + throw customError; + }, + }); + await expect(getCourseLogoOrg()).rejects.toThrow(customError); + }); +}); From 3cf5f71a03f9eca747a7574dd2337aada1291715 Mon Sep 17 00:00:00 2001 From: Sandro Costa Date: Tue, 19 Mar 2024 11:57:17 +0000 Subject: [PATCH 2/5] fix: organization logo display * fix: platform logo on mobile display * fix: hide course name on mobile --- src/index.scss | 6 ++--- src/learning-header/LearningHeader.jsx | 28 +++++++++++++-------- src/learning-header/LearningHeader.test.jsx | 1 - src/learning-header/_header.scss | 10 ++++++++ 4 files changed, 30 insertions(+), 15 deletions(-) create mode 100644 src/learning-header/_header.scss diff --git a/src/index.scss b/src/index.scss index 94114bd44..17a2c2e7c 100644 --- a/src/index.scss +++ b/src/index.scss @@ -43,9 +43,9 @@ $white: #fff; .user-dropdown { .btn { height: 3rem; - // @media (max-width: -1 + map-get($grid-breakpoints, "sm")) { - // padding: 0 0.5rem; - // } + @media (map-get($grid-breakpoints, "sm")) { + padding: 0 0.5rem; + } } } } diff --git a/src/learning-header/LearningHeader.jsx b/src/learning-header/LearningHeader.jsx index feae1cfab..d8fe165b7 100644 --- a/src/learning-header/LearningHeader.jsx +++ b/src/learning-header/LearningHeader.jsx @@ -26,6 +26,10 @@ LinkedLogo.propTypes = { alt: PropTypes.string.isRequired, }; +// this feature flag is not included on the frontend-platform, we have to get it directly from ENV +const enabledOrgLogo = process.env.ENABLED_ORG_LOGO || false; + + const LearningHeader = ({ courseOrg, courseTitle, intl, showUserDropdown, }) => { @@ -52,17 +56,19 @@ const LearningHeader = ({ {intl.formatMessage(messages.skipNavLink)}
{headerLogo} -
- { - (courseOrg && logoOrg) - && {`${courseOrg} - } - - {courseTitle} - +
+
+ {enabledOrgLogo ? ( + (courseOrg && logoOrg) + && {`${courseOrg} + ) : null} + + {courseTitle} + +
{showUserDropdown && authenticatedUser && ( { render(
); waitFor( () => { - expect(screen.getByAltText(`${courseData.courseOrg} logo`)).toHaveAttribute('src', 'logo-url'); expect(screen.getByText(`${courseData.courseOrg}`)).toBeInTheDocument(); expect(screen.getByText(courseData.courseTitle)).toBeInTheDocument(); diff --git a/src/learning-header/_header.scss b/src/learning-header/_header.scss new file mode 100644 index 000000000..357cc6b3a --- /dev/null +++ b/src/learning-header/_header.scss @@ -0,0 +1,10 @@ +@import "../../node_modules/bootstrap/scss/bootstrap-grid"; +@import "../../node_modules/bootstrap/scss/mixins/breakpoints"; + +.logo { + img { + @include media-breakpoint-down(sm) { + max-width: 85% !important; + } + } +} \ No newline at end of file From 736a5c793cf45e4e7af2dbbbc0f2cddabc4e67ae Mon Sep 17 00:00:00 2001 From: Diana Catalina Olarte Date: Tue, 25 Jun 2024 22:45:42 +1000 Subject: [PATCH 3/5] style: fix lint issues --- src/learning-header/LearningHeader.jsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/learning-header/LearningHeader.jsx b/src/learning-header/LearningHeader.jsx index d8fe165b7..5e5c7d870 100644 --- a/src/learning-header/LearningHeader.jsx +++ b/src/learning-header/LearningHeader.jsx @@ -29,7 +29,6 @@ LinkedLogo.propTypes = { // this feature flag is not included on the frontend-platform, we have to get it directly from ENV const enabledOrgLogo = process.env.ENABLED_ORG_LOGO || false; - const LearningHeader = ({ courseOrg, courseTitle, intl, showUserDropdown, }) => { @@ -40,7 +39,7 @@ const LearningHeader = ({ if (courseOrg) { getCourseLogoOrg().then((logoOrgUrl) => { setLogoOrg(logoOrgUrl); }); } - }, []); + }); const headerLogo = ( Date: Mon, 1 Jul 2024 21:11:32 +1000 Subject: [PATCH 4/5] fix: ENABLED_ORG_LOGO definition --- src/Header.jsx | 1 + src/learning-header/LearningHeader.jsx | 5 +---- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Header.jsx b/src/Header.jsx index bff801fe0..d23098244 100644 --- a/src/Header.jsx +++ b/src/Header.jsx @@ -28,6 +28,7 @@ ensureConfig([ subscribe(APP_CONFIG_INITIALIZED, () => { mergeConfig({ AUTHN_MINIMAL_HEADER: !!process.env.AUTHN_MINIMAL_HEADER, + ENABLED_ORG_LOGO: !!process.env.ENABLED_ORG_LOGO, }, 'Header additional config'); }); diff --git a/src/learning-header/LearningHeader.jsx b/src/learning-header/LearningHeader.jsx index 5e5c7d870..2d30418dd 100644 --- a/src/learning-header/LearningHeader.jsx +++ b/src/learning-header/LearningHeader.jsx @@ -25,15 +25,12 @@ LinkedLogo.propTypes = { src: PropTypes.string.isRequired, alt: PropTypes.string.isRequired, }; - -// this feature flag is not included on the frontend-platform, we have to get it directly from ENV -const enabledOrgLogo = process.env.ENABLED_ORG_LOGO || false; - const LearningHeader = ({ courseOrg, courseTitle, intl, showUserDropdown, }) => { const { authenticatedUser } = useContext(AppContext); const [logoOrg, setLogoOrg] = useState(null); + const enabledOrgLogo = getConfig().ENABLED_ORG_LOGO || false; useEffect(() => { if (courseOrg) { From 9d8b7a9d45ca1a24e8f71a249958dea5470960eb Mon Sep 17 00:00:00 2001 From: Sandro Costa Date: Tue, 20 Aug 2024 11:27:22 +0000 Subject: [PATCH 5/5] fix: rename ENABLED_ORG_LOGO setting Rename the setting to be in the present tense, as a way to make it clearer. The setting is now called `ENABLE_ORG_LOGO`. --- src/Header.jsx | 2 +- src/learning-header/LearningHeader.jsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Header.jsx b/src/Header.jsx index d23098244..38f1307f8 100644 --- a/src/Header.jsx +++ b/src/Header.jsx @@ -28,7 +28,7 @@ ensureConfig([ subscribe(APP_CONFIG_INITIALIZED, () => { mergeConfig({ AUTHN_MINIMAL_HEADER: !!process.env.AUTHN_MINIMAL_HEADER, - ENABLED_ORG_LOGO: !!process.env.ENABLED_ORG_LOGO, + ENABLE_ORG_LOGO: !!process.env.ENABLE_ORG_LOGO, }, 'Header additional config'); }); diff --git a/src/learning-header/LearningHeader.jsx b/src/learning-header/LearningHeader.jsx index 2d30418dd..779589fb2 100644 --- a/src/learning-header/LearningHeader.jsx +++ b/src/learning-header/LearningHeader.jsx @@ -30,7 +30,7 @@ const LearningHeader = ({ }) => { const { authenticatedUser } = useContext(AppContext); const [logoOrg, setLogoOrg] = useState(null); - const enabledOrgLogo = getConfig().ENABLED_ORG_LOGO || false; + const enableOrgLogo = getConfig().ENABLE_ORG_LOGO || false; useEffect(() => { if (courseOrg) { @@ -53,8 +53,8 @@ const LearningHeader = ({
{headerLogo}
-
- {enabledOrgLogo ? ( +
+ {enableOrgLogo ? ( (courseOrg && logoOrg) && {`${courseOrg} ) : null}