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

[FC-0056] Course outline sidebar #1342

Closed
wants to merge 19 commits into from

Conversation

ihor-romaniuk
Copy link
Contributor

@ihor-romaniuk ihor-romaniuk commented Apr 1, 2024

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: ts-develop

Description

This pull request adds an important feature to our platform: displaying a navigation sidebar within a given course.

Design

https://www.figma.com/file/gew5tORDX4Q7wxOS8vjqZu/side-nav-OEX?type=design&node-id=318-3234&mode=design&t=rBe1ToNYP8RY6QOp-0

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. Go to Course Unit page from the Course Outline page.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Apr 1, 2024
@openedx-webhooks
Copy link

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.

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.

* feat: [AXIMST-572] create course-outline sidebar

* fix: [AXIMST-572] after review
* feat: [AXIMST-617] display section and sequence sidebar level

* fix: after review

* fix: after review
* feat: [AXIMST-590] display units on sidebar, two tier layout

* fix: expand prop-types

* fix: after review
* feat: [AXIMST-578] display outline sidebar depends on feature flag

* fix: after review
* feat: [AXIMST-635] display discussions sidebar by waffle flag

* fix: after demo

* fix: after review

* fix: upstream failed tests
* feat: [AXIMST-641] display special exam label and unit lock icon

* fix: after backend changes

* fix: error with entrance exam

* fix: after review
* feat: make sidebar fixed on scroll and add tests

* fix: extend tests

* fix: tests

* fix: codecov

* fix: after review
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 97.51553% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 88.68%. Comparing base (d4e7b41) to head (96c0e0f).
Report is 16 commits behind head on master.

❗ Current head 96c0e0f differs from pull request most recent head 110964d. Consider uploading reports for the commit 110964d to get more accurate results

Files Patch % Lines
...ebar/sidebars/course-outline/CourseOutlineTray.jsx 93.87% 3 Missing ⚠️
...se/sidebar/sidebars/course-outline/SidebarUnit.jsx 81.81% 2 Missing ⚠️
src/courseware/data/utils.js 96.00% 2 Missing ⚠️
src/courseware/data/slice.js 97.14% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1342      +/-   ##
==========================================
+ Coverage   88.24%   88.68%   +0.44%     
==========================================
  Files         293      311      +18     
  Lines        4992     5234     +242     
  Branches     1266     1330      +64     
==========================================
+ Hits         4405     4642     +237     
- Misses        571      576       +5     
  Partials       16       16              

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

…plete subsection (#28)

* fix: [AXIMST-748] update outline sidebar for locked subsection on complete subsection

* fix: tests

* fix: after review
@itsjeyd
Copy link

itsjeyd commented Apr 12, 2024

Hey @jmakowski1123, since this is for an FC project with status In Development, can we consider it approved from the product perspective?

Same question for other PRs related to the FC-0056 project.

CC @mphilbrick211 @crathbun428

)

* feat: [AXIMST-702] display completions for sections and subsections

* fix: [AXIMST-702] after review

* fix: [AXIMST-702] after review
@itsjeyd itsjeyd added the product review PR requires product review before merging label Apr 22, 2024
* feat: [AXIMST-708] hide horizontal navigation in units

* fix: after review

* fix: [AXIMST-708] fix typo

---------

Co-authored-by: ihor-romaniuk <[email protected]>
@mphilbrick211
Copy link

Hey @jmakowski1123, since this is for an FC project with status In Development, can we consider it approved from the product perspective?

Same question for other PRs related to the FC-0056 project.

CC @mphilbrick211 @crathbun428

@jmakowski1123 just following up on this :)

@crathbun428
Copy link

crathbun428 commented Apr 23, 2024

@mphilbrick211 - If this pull request just represents the Phase 1 portion of the project, which has been tested, then yes, this has been approved by Product. The Phase 2 of the FC has yet to be tested by product.

@GlugovGrGlib - does this pull request just represent the Phase 1 portion of the project?

@GlugovGrGlib
Copy link
Member

does this pull request just represent the Phase 1 portion of the project?

@crathbun428 yes, all FC-0056 PR, that are currently opened to openedx repositories are part of the phase 1

@crathbun428
Copy link

@GlugovGrGlib - Excellent - thank you for confirming.

@mphilbrick211 - We can consider this approved by product. Please let me know if you have any follow-up questions!

@itsjeyd
Copy link

itsjeyd commented Apr 26, 2024

Thanks @crathbun428 @GlugovGrGlib @mphilbrick211 for sorting this out.

@openedx-webhooks
Copy link

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

@ihor-romaniuk
Copy link
Contributor Author

The main PR about the course outline sidebar functionality #1349

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U product review PR requires product review before merging
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants