asadiqbal08/Removed course header from Learning MFE

[asadiqbal08]

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

Fixes-263

Blocked By:

• issue-252
• PR-133

[openedx-webhooks]

Thanks for the pull request, @asadiqbal08! I've created OSPR-6191 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[natabene]

@asadiqbal08 Thank you for your contribution. Please let me know once it is ready for our review.

[davidjoy]

This looks great - so I think we're going to need to get the frontend-component-header-edx portion of this done before I'd feel comfortable merging.

[asadiqbal08]

@davidjoy should I created a corresponding PR to frontend-component-header-edx or someone else is taking care of it ?

[asadiqbal08]

@davidjoy another PR for your reference

[mikix]

Thank you!

[codecov]

Codecov Report

Merging #715 (943d46c) into master (cc0c3c2) will increase coverage by 0.67%.
The diff coverage is 100.00%.

❗ Current head 943d46c differs from pull request most recent head 90a225a. Consider uploading reports for the commit 90a225a to get more accurate results

@@            Coverage Diff             @@
##           master     #715      +/-   ##
==========================================
+ Coverage   83.76%   84.44%   +0.67%     
==========================================
  Files         256      241      -15     
  Lines        4423     4224     -199     
  Branches     1100     1073      -27     
==========================================
- Hits         3705     3567     -138     
+ Misses        697      637      -60     
+ Partials       21       20       -1     

Impacted Files	Coverage Δ
...c/course-home/goal-unsubscribe/GoalUnsubscribe.jsx	100.00% <ø> (ø)
src/course-tabs/CourseTabsNavigation.jsx	100.00% <ø> (ø)
src/generic/messages.js	100.00% <ø> (ø)
src/tab-page/LoadedTabPage.jsx	100.00% <ø> (ø)
src/tab-page/TabPage.jsx	94.73% <ø> (-0.27%)	⬇️
src/course-tabs/messages.js	100.00% <100.00%> (ø)
src/course-home/data/api.js	86.08% <0.00%> (-0.52%)	⬇️
src/store.js	100.00% <0.00%> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc0c3c2...90a225a. Read the comment docs.

[mikix]

Looks like tests pass, but can you fix your commit header to be like: feat: removed course header from Learning MFE or something to satisfy our commit linter?

[asadiqbal08]

@mikix @davidjoy I was rebasing this PR with latest and came to know that a recent PR -- engage product tour has merged that contains some changes in components of course-header that is going to be removed from this repository with this PR.
Please take a look on the change set and let me know what to do ?

• Should I need to create corresponding PRs in frontend-component-header and frontend-component-header-edx to keep the changes of product tour
• OR
• Should We request the author of PR -- engage product tour to move logic in LearningHeader of those repositories and revert those from this repo ?

[ciduarte]

@asadiqbal08 I just put up a PR to remove tour logic from the Header component! Sorry for the mixup, you should be able to rebase and carry on when it's merged.

edit: Just merged!

[openedx-webhooks]

@asadiqbal08 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.