-
Notifications
You must be signed in to change notification settings - Fork 10
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
[BD-14] Add StudioHeader header variant #102
[BD-14] Add StudioHeader header variant #102
Conversation
…nt property to Menu
Thanks for the pull request, @gabor-boros! I've created BLENDED-626 to keep track of it in Jira. More details are on the BD-14 project page. This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate. |
@marcotuts Is this a Blended ticket? |
@natabene Yes it is, BD-14. Just updated the title. |
Thanks for your work so far on this. We're pausing development and code review on BD-14 for now to allow more planning and design to be done before work is resumed on Milestone 3. Until then, edX most likely won't be able to review this PR. |
@davidjoy - can you clarify if we've taken a different approach here? this has been sitting for a while but the team working on BD-14 was looking to pick this up soon |
In general, yes, this is still the approach. The point of adding the StudioHeader in this repository is to provide an edx.org specific override to the generic StudioHeader added to frontend-component-header. If that header doesn't exist, it'll need to be added and used in the MFE first before adding anything here will be meaningful. The point of adding the header to frontend-component-header is so that Open edX operators can override it with their own custom version - including us. Since this PR was created, a team added the Learning MFE's header to this repo and to frontend-component-header. I would suggest modeling the directory structure/exports of this PR after that header for consistency. That might require a little refactoring in this PR; I'm not quite sure how different they are. But it shouldn't be that bad regardless. |
@arbrandes Since openedx/frontend-app-library-authoring#18 is closed (openedx/frontend-app-library-authoring#36 (comment)), should we close this too or is this something to keep? |
Like openedx/frontend-platform#116, this can be closed. |
@gabor-boros Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
This PR adds a new shared header component, which is based on the already existing
Header
and can be used across Studio related pages and frontend apps like the library authoring app.Also, from this point, other header variants could be created easily. Besides that, those components/pages using this new
StudioHeader
variant can add children components if required.JIRA tickets: TNL-7403 / TNL-7503
Discussions: Link to any public dicussions about this PR or the design/architecture. Otherwise omit this.
Dependencies:
Screenshots:
The new header in use:
Sandbox URL: N/A
Merge deadline: None
Testing instructions:
Author notes and concerns:
Reviewers