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

feat: [FC-0044] Unit page - refactoring unit page breadcrumbs and actions #827

Conversation

ihor-romaniuk
Copy link
Contributor

@ihor-romaniuk ihor-romaniuk commented Feb 8, 2024

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/openedx/edx-platform.git
EDX_PLATFORM_VERSION: master

TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_unit_page
    everyone: true

TUTOR_GROVE_MFE_LMS_COMMON_SETTINGS:
  MFE_CONFIG:
    ENABLE_UNIT_PAGE: true

Description

This pull request addresses the refactoring of the breadcrumbs section and the preview buttons ("View live version" and "Preview") within the Unit page. The primary focus is on simplifying the existing codebase and optimizing the usage of data retrieved from the API endpoint.

Issue: openedx/platform-roadmap#321

Benefits

  • Code simplicity: The refactoring results in a more straightforward and easy-to-understand codebase, promoting maintainability and future development.
  • Optimized data usage: Using existing data from the API endpoint, we unify data usage across the application, improving application performance.

Developer notes

The first three commits in this pull request are temporary. Once the #809 is merged, the fourth commit of this pull request will become the main commit.

Design

https://www.figma.com/file/YeKFwSpyLaJFDs3f3p3TSa/Studio-1%3A1-mock-ups?node-id=599%3A23595

image

Testing instructions

  1. Run master devstack.
  2. Start platform make dev.up.lms+cms+frontend-app-course-authoring and make checkout on this branch.
  3. Enable new Unit page by adding a waffle flad contentstore.new_studio_mfe.use_new_unit_page in CMS admin panel.
  4. Go to Course Unit page from the Course Outline page.

@openedx-webhooks
Copy link

openedx-webhooks commented Feb 8, 2024

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:

  • 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.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 8, 2024
@ihor-romaniuk ihor-romaniuk marked this pull request as ready for review February 8, 2024 10:33
@ihor-romaniuk ihor-romaniuk force-pushed the romaniuk/unit-page/refactoring-unit-page-breadcrumbs-and-actions branch from 99c9339 to fa50334 Compare February 8, 2024 10:57
Copy link

codecov bot commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1555e9f) 89.13% compared to head (8faae33) 89.10%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #827      +/-   ##
==========================================
- Coverage   89.13%   89.10%   -0.04%     
==========================================
  Files         548      547       -1     
  Lines        9647     9629      -18     
  Branches     2067     2061       -6     
==========================================
- Hits         8599     8580      -19     
- Misses       1001     1002       +1     
  Partials       47       47              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ihor-romaniuk ihor-romaniuk force-pushed the romaniuk/unit-page/refactoring-unit-page-breadcrumbs-and-actions branch 2 times, most recently from 23aa273 to 54f1797 Compare February 9, 2024 14:42
@KristinAoki
Copy link
Member

Just need to create sandbox and test it on there. After checking it works there, the PR can be merged

@ihor-romaniuk ihor-romaniuk force-pushed the romaniuk/unit-page/refactoring-unit-page-breadcrumbs-and-actions branch from 54f1797 to cd2d0bf Compare February 12, 2024 08:10
@ihor-romaniuk ihor-romaniuk added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Feb 12, 2024
@ihor-romaniuk
Copy link
Contributor Author

@arbrandes I tried to create a sandbox as you recommended but faced with some problem.
Could you check what can be wrong, please?

@ihor-romaniuk
Copy link
Contributor Author

@KristinAoki Sandbox is ready for testing.

@KristinAoki
Copy link
Member

In the sandbox there is a weird whitebox that surround the breadcrumbs and header actions.
Screenshot 2024-02-13 at 10 19 22 AM

@ihor-romaniuk ihor-romaniuk added create-sandbox open-craft-grove should create a sandbox environment from this PR and removed create-sandbox open-craft-grove should create a sandbox environment from this PR labels Feb 14, 2024
@KristinAoki KristinAoki merged commit 51c5f9c into openedx:master Feb 14, 2024
5 of 6 checks passed
@openedx-webhooks
Copy link

@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.

@ihor-romaniuk ihor-romaniuk deleted the romaniuk/unit-page/refactoring-unit-page-breadcrumbs-and-actions branch February 15, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-sandbox open-craft-grove should create a sandbox environment from this PR open-source-contribution PR author is not from Axim or 2U
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants