Introduced a new header as LearningHeader from Frontend Learning MFE. #1
Conversation
|
@pdpinch anyone from your team can also take a look before I created an upstream PR to edX ? |
|
In the description you say:
Have you opened a PR that does that? |
src/index.scss
Outdated
| @@ -25,6 +25,32 @@ $white: #fff; | |||
| } | |||
| } | |||
|
|
|||
| .learning-header { | |||
| min-width: 0; | |||
| background-color: red; | |||
There was a problem hiding this comment.
Red background? Is that intentional or left over from testing? :)
src/learning-header/Header.jsx
Outdated
| }) { | ||
| const { authenticatedUser } = useContext(AppContext); | ||
|
|
||
| const { enterpriseLearnerPortalLink, enterpriseCustomerBrandingConfig } = useEnterpriseConfig( |
There was a problem hiding this comment.
I would suggest we remove this enterprise-specific code and get rid of the dependency on @edx/frontend-enterprise-utils. I don't think that's something that the community would want, in general, so we probably don't want it in the default header. It's probably only used by edx.org.
edx.org can add it in to frontend-component-header-edx's version of the LearningHeader. CC: @adamstankiewicz - who will want to know about that decision/may have input.
|
@davidjoy Thanks for putting suggestions over the PR. I have removed the enterprise specific stuff but I believe, enterprise related stuff should be the part of edx/frontend-component-header-edx so I will create another ticket to keep track of this. Thoughts ? |
No @pdpinch , As that stuff is dependent upon this PR. |
That sounds great, thanks! Also, my expectation is that it's perfectly fair for us to ask someone from edX to update frontend-component-header-edx. 😄 But documenting the delta as you pull things out would be very helpful for that effort. |
asadiqbal08
force-pushed
the
asadiqbal08/fix-252
branch
from
e26aa41 to
2ab3e34
Compare
|
Closing this in favor of this PR openedx#133 |
fixes: mitodl/mitxonline#252
As per some suggestion by David, We need to refactor the code in EdX Learning MFE to utilize the header from frontend-component-header instead of course-header
.