Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Learning MFE Header to frontend-component-header MFE #252

Closed
5 tasks
asadiqbal08 opened this issue Oct 7, 2021 · 4 comments · Fixed by openedx/frontend-component-header#133
Closed
5 tasks
Assignees

Comments

@asadiqbal08
Copy link
Contributor

asadiqbal08 commented Oct 7, 2021

Related to #112

As per some suggestion by David, We need to refactor the code in EdX Learning MFE to utlize the header from frontend-component-header instead of course-header

Acceptance Criteria:

Create an upstream PR to edx frontend-component-header repository with the following criteria

#### Once the Code integrated in frontend-component-header and frontend-component-header-edx
#### NOTE: We can move the below details into another story.

Create an upstream PR to frontend-app-learning repository with the following criteria:
- [ ] The code related to course-header should be removed from the repository.
- [ ] Introduced the @edx/frontend-component-header in package.json.
- [ ] Import the header import { LearningHeader as Header } from '@edx/frontend-component-header'; instead of import { Header } from '../course-header';

NOTE: After that, we will be able to use the npm aliases as described.~~

Out of Scope

  • ?????
@asadiqbal08 asadiqbal08 changed the title Move Learning MFE Header to frontend-component-header Move Learning MFE Header to frontend-component-header MFE Oct 7, 2021
@asadiqbal08
Copy link
Contributor Author

@pdpinch FYI

@davidjoy
Copy link

Hi there - I'd suggest that CourseTabsNavigation stay in the learning MFE - its very specific to that application and is used outside/below the main header. I think we want to limit what we pull over to frontend-component-header to be the top-most Header. Most of the sub-components of it will be able to share code with the other Headers we put in frontend-component-header as we go forward.

Feel free to tag me if I can help answer questions here - this was brought up in the Frontend Working Group's meeting today, where we were pleased to see this issue!

@davidjoy
Copy link

Another note - I think if we leave CourseTabsNavigation behind, we can get away without moving anything from generic as well. If we have things in generic that we really want to start using across MFEs or libraries, we may want to consider moving them to https://github.com/edx/paragon rather than duplicating them into frontend-component-header.

@asadiqbal08
Copy link
Contributor Author

@davidjoy Thanks for the input. I will try to create an upstream PR first for the some possible changes in frontend-component-header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment