feat: [FC-0044] Unit page - display xblock components #857
Conversation
|
Thanks for the pull request, @ihor-romaniuk! 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:
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. |
openedx-webhooks
added
the
open-source-contribution
arbrandes
added
the
create-sandbox
ihor-romaniuk
force-pushed
the
romaniuk/unit-page/display-xblock-components
branch
3 times, most recently
from
ce76609 to
e1c89b0
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #857 +/- ##
=======================================
Coverage 90.64% 90.64%
=======================================
Files 578 578
Lines 10037 10037
Branches 2142 2142
=======================================
Hits 9098 9098
Misses 899 899
Partials 40 40 ☔ View full report in Codecov by Sentry. |
|
Sandbox deployment successful 🚀 |
ihor-romaniuk
force-pushed
the
romaniuk/unit-page/display-xblock-components
branch
from
e1c89b0 to
62ae4b5
Compare
ihor-romaniuk
added
create-sandbox
|
Sandbox deployment successful 🚀 |
ihor-romaniuk
added
create-sandbox
package.json
Outdated
| @@ -46,6 +46,7 @@ | |||
| "@edx/frontend-platform": "7.0.1", | |||
| "@edx/openedx-atlas": "^0.6.0", | |||
| "@openedx/paragon": "^21.5.7", | |||
| "@edx/react-unit-test-utils": "^1.7.0", | |||
There was a problem hiding this comment.
I'd rather we stuck to vanilla react-testling-library, if at all possible. I'm not sure if react-unit-test-utils is maintained anymore. Does it make our lives much easier?
There was a problem hiding this comment.
This part of the code has been removed for this MR.
In the future, this functionality will be added to further MPs with appropriate changes.
| .unit-iframe-wrapper .alert-danger { | ||
| margin-bottom: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can we rename this file to CourseXBlock.scss, please?
|
Got an error in testing in the sandbox.
|
|
Sandbox deployment successful 🚀 |
|
Sandbox deployment successful 🚀 |
|
Sandbox deployment successful 🚀 |
| const messages = defineMessages({ | ||
| blockAltButtonEdit: { | ||
| id: 'course-authoring.course-unit.xblock.button.edit.alt', | ||
| defaultMessage: 'Edit Item', |
There was a problem hiding this comment.
| defaultMessage: 'Edit Item', | |
| defaultMessage: 'Edit', |
| alt={intl.formatMessage(messages.blockAltButtonEdit)} | ||
| iconAs={EditIcon} | ||
| size="md" | ||
| onClick={() => {}} |
There was a problem hiding this comment.
Is this being handled in another PR?
There was a problem hiding this comment.
Yes, it will be realized in the future iteration of making Edit button functional
| size="md" | ||
| onClick={() => {}} | ||
| /> | ||
| <Dropdown> |
There was a problem hiding this comment.
The functionality of the dropdown is great, but the text color should not be colored like it is a link. Additionally the hover state for the dropdown item should not be blue. Currently it creates a contrast error which is an accessibility concern.
There was a problem hiding this comment.
It causes because to override custom styles which were added to this instance edx-simple-theme/common/_overrides.scss and it affects the all dropdowns on the page. I think, we need to make changes in the _overrides.scss file instead of adding new extra styles of this page.
| <Dropdown.Item> | ||
| {intl.formatMessage(messages.blockLabelButtonMove)} | ||
| </Dropdown.Item> | ||
| <Dropdown.Item> |
There was a problem hiding this comment.
Confirming that this functionality will be addressed in another PR
There was a problem hiding this comment.
You are right, it will be done in next iterations.
| <Dropdown.Item onClick={() => unitXBlockActions.handleDuplicate(id)}> | ||
| {intl.formatMessage(messages.blockLabelButtonDuplicate)} | ||
| </Dropdown.Item> | ||
| <Dropdown.Item> |
There was a problem hiding this comment.
Confirming that this functionality will be addressed in another PR
There was a problem hiding this comment.
Requested mocks for Jon Fay for the move modal because there are none in th current Figma
There was a problem hiding this comment.
Yes, it will be done in next iterations.
| return ( | ||
| <div ref={courseXBlockElementRef} {...props}> | ||
| <Card className="mb-1"> | ||
| <Card.Header |
There was a problem hiding this comment.
Missing drag handle. Is that in scope for this ticket?
There was a problem hiding this comment.
No, the Drag&Drop functionality will be done in next iterations.
|
This is now working in the sandbox! A couple of initial observations:
Let me know if any of these are out of scope to be addressed or implemented here. |
|
Sandbox deployment successful 🚀 |
Text, Video, and Problem blocks automatically open their new editors (removing the uneccessary extra edit click). The new problem editor contains the ability to chose the different problem types. |
ihor-romaniuk
force-pushed
the
romaniuk/unit-page/display-xblock-components
branch
from
1cb7f79 to
32cfc78
Compare
ihor-romaniuk
added
create-sandbox
|
Sandbox deployment failed 💥 |
That makes sense. Should they be working, then, though? |
|
@arbrandes It should work. |
ihor-romaniuk
added
create-sandbox
GlugovGrGlib
added
create-sandbox
ihor-romaniuk
added
create-sandbox
|
Sandbox deployment successful 🚀 |
|
Hi @arbrandes @KristinAoki |
ihor-romaniuk
force-pushed
the
romaniuk/unit-page/display-xblock-components
branch
from
32cfc78 to
93e0c27
Compare
ihor-romaniuk
force-pushed
the
romaniuk/unit-page/display-xblock-components
branch
from
93e0c27 to
c8c8cad
Compare
|
Sandbox deployment successful 🚀 |
|
@ihor-romaniuk 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Settings
Description
This merge request introduces a new feature that enhances page rendering by incorporating dynamic components. The key components include:
Component Header:
Component Content:
As a first iteration, xblock content is displayed as a placeholder block. The next iteration will change the rendering of the content by displaying HTML and resources received from the legacy endpoint (not using an iframe).
Issue: openedx/platform-roadmap#321
Depends on:
Design
https://www.figma.com/file/YeKFwSpyLaJFDs3f3p3TSa/Studio-1%3A1-mock-ups?node-id=599%3A23595
Testing instructions
contentstore.new_studio_mfe.use_new_unit_pagein CMS admin panel.