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: course outline page #694

Merged

Conversation

navinkarkera
Copy link
Contributor

@navinkarkera navinkarkera commented Nov 17, 2023

Continuing work from #598

This PR adds

  • Top level page
  • Status bar (collapse/expand will be completed after section list is implemented.)
  • Reindex button
  • View Live link
  • Enable course highlights button
  • Start date link
  • Checklists link

Screenshot:

image

Depends on: openedx/edx-platform#33667

Testing instructions:

@openedx-webhooks
Copy link

Thanks for the pull request, @navinkarkera! 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 Nov 17, 2023
@navinkarkera navinkarkera changed the title feat: create course outline page feat: course outline page Nov 17, 2023
Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 28 lines in your changes are missing coverage. Please review.

Comparison is base (bebbc15) 88.65% compared to head (80cc5e3) 88.85%.

Files Patch % Lines
src/course-outline/hooks.jsx 66.66% 12 Missing ⚠️
src/course-outline/utils/getChecklistValues.js 85.71% 6 Missing ⚠️
src/hooks.js 50.00% 3 Missing and 1 partial ⚠️
src/course-outline/data/thunk.js 95.55% 2 Missing ⚠️
...e-outline/header-navigations/HeaderNavigations.jsx 86.66% 1 Missing and 1 partial ⚠️
...c/course-outline/utils/getChecklistForStatusBar.js 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #694      +/-   ##
==========================================
+ Coverage   88.65%   88.85%   +0.20%     
==========================================
  Files         447      468      +21     
  Lines        6846     7168     +322     
  Branches     1461     1539      +78     
==========================================
+ Hits         6069     6369     +300     
- Misses        752      772      +20     
- Partials       25       27       +2     

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

@navinkarkera navinkarkera force-pushed the navin/course-outline-mfe-page branch from 2f3b953 to 543b171 Compare November 22, 2023 06:30
@navinkarkera navinkarkera force-pushed the navin/course-outline-mfe-page branch 2 times, most recently from 1fe7488 to ded2c81 Compare November 24, 2023 05:35
Copy link
Contributor

@Cup0fCoffee Cup0fCoffee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Some minor suggestions, but otherwise LGTM.

  • I tested this
  • I read through the code
  • [-] I checked for accessibility issues
  • [-] Includes documentation

src/course-outline/CourseOutline.test.jsx Outdated Show resolved Hide resolved
src/course-outline/CourseOutline.test.jsx Outdated Show resolved Hide resolved
src/course-outline/CourseOutline.test.jsx Outdated Show resolved Hide resolved
src/course-outline/outline-sidebar/OulineSidebar.scss Outdated Show resolved Hide resolved
src/course-outline/outline-sidebar/OutlineSidebar.test.jsx Outdated Show resolved Hide resolved
src/course-outline/outline-sidebar/OutlineSidebar.test.jsx Outdated Show resolved Hide resolved
src/course-outline/status-bar/StatusBar.jsx Outdated Show resolved Hide resolved
@navinkarkera
Copy link
Contributor Author

@Cup0fCoffee Thanks for the thorough review. Addressed all your comments.

@navinkarkera navinkarkera added the jira:2u We want an issue in the 2U Jira instance label Nov 27, 2023
@openedx-webhooks
Copy link

I've created issue TNL-11237 in the private 2U Jira.

src/course-outline/enable-highlights-modal/messages.js Outdated Show resolved Hide resolved
src/course-outline/status-bar/messages.js Outdated Show resolved Hide resolved
src/course-outline/status-bar/messages.js Outdated Show resolved Hide resolved
src/CourseAuthoringRoutes.jsx Outdated Show resolved Hide resolved
src/header/utils.js Outdated Show resolved Hide resolved
@navinkarkera navinkarkera force-pushed the navin/course-outline-mfe-page branch from 7ce8fa8 to 5ddb9bf Compare December 5, 2023 10:41
Copy link
Member

@KristinAoki KristinAoki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to make the one message change that I previously requested. After that I will merge this PR.

vladislavkeblysh and others added 2 commits December 6, 2023 20:09
* feat: [2u-259] add components

* feat: [2u-259] fix sidebar

* feat: [2u-259] add tests, fix links

* feat: [2u-259] fix messages

* feat: [2u-159] fix reducer and sidebar

* feat: [2u-259] fix reducer

* feat: [2u-259] remove warning from selectors

* feat: [2u-259] remove indents

---------

Co-authored-by: Vladislav Keblysh <[email protected]>

feat: Course outline Status Bar (#50)

* feat: [2u-259] add components

* feat: [2u-259] fix sidebar

* feat: [2u-259] add tests, fix links

* feat: [2u-259] fix messages

* feat: [2u-159] fix reducer and sidebar

* feat: add checklist

* feat: [2u-259] fix reducer

* feat: [2u-259] remove warning from selectors

* feat: [2u-259] remove indents

* feat: [2u-259] add api, enable modal

* feat: [2u-259] add tests

* feat: [2u-259] add translates

* feat: [2u-271] fix transalates

* feat: [2u-281] fix isQuery pending, utils, hooks

* feat: [2u-281] fix useScrollToHashElement

* feat: [2u-271] fix imports

---------

Co-authored-by: Vladislav Keblysh <[email protected]>

feat: Course Outline Reindex (#55)

* feat: [2u-277] add alerts

* feat: [2u-277] add translates

* feat: [2u-277] fix tests

* fix: [2u-277] fix slice and hook

---------

Co-authored-by: Vladislav Keblysh <[email protected]>

fix: Course outline tests (#56)

* fix: fixed course outline status bar tests

* fix: fixed course outline status bar tests

* fix: fixed course outline enable highlights modal tests

* fix: enable modal tests

fix: increase code coverage on the page
feat: lms live link

chore: update outline link

fix: course outline link

refactor: remove unnecessary css and rename test file

refactor: remove unnecessary css from outlineSidebar

test: make use of message variable instead of hardcoded text

refactor: remove unnecessary h5 class

test: use test id for detecting component

refactor: update course outline url and some default messages
@navinkarkera navinkarkera force-pushed the navin/course-outline-mfe-page branch from 2875840 to 80cc5e3 Compare December 6, 2023 14:39
@navinkarkera
Copy link
Contributor Author

Just need to make the one message change that I previously requested. After that I will merge this PR.

@KristinAoki Done. Also squashed commits, this is ready to be merged.

@KristinAoki KristinAoki merged commit 04c1427 into openedx:master Dec 6, 2023
6 checks passed
@openedx-webhooks
Copy link

@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira:2u We want an issue in the 2U Jira instance 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.

5 participants