Skip to content

feat: Added Schedule and Details MFE page - #547

Merged
KristinAoki merged 16 commits into
openedx:masterfrom
raccoongang:ruzniaievdm/feat/schedule-and-details-mfe
Aug 8, 2023
Merged

feat: Added Schedule and Details MFE page#547
KristinAoki merged 16 commits into
openedx:masterfrom
raccoongang:ruzniaievdm/feat/schedule-and-details-mfe

Conversation

@ruzniaievdm

@ruzniaievdm ruzniaievdm commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Description

  • Created a new page for Schedule and details
  • Added basic, credit, details, instructors, introducing, learning outcomes, license, pacing, requirements and schedule sections
  • Added WysiwygEditor
  • Fixed bugs
  • Added internet connection error

Please merge it after

  • frontend-lib-content-components PR
  • edx-platform PR

* feat: Create MFE page with base structure for schedule and details
* feat: Add Course schedule section to the MFE page

* fix: after demo and resolve discussions

* resolve discussions round 2
#12)

* feat: Add default views for Course Details and Introduce Your Course sections

* fix: resolve discussions
* feat: Add Requirements and Licensing section

* fix: add stateful button for saving advanced settings
@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch 2 times, most recently from d33b237 to c14da57 Compare August 1, 2023 15:57
@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch 2 times, most recently from 48f492c to 88981a3 Compare August 1, 2023 16:51
@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch from 88981a3 to 29fb06e Compare August 1, 2023 17:13
@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch 6 times, most recently from 8e93a8e to 8ca4e48 Compare August 3, 2023 09:13
@codecov

codecov Bot commented Aug 3, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 82.54% and project coverage change: +1.83% 🎉

Comparison is base (9f4422d) 79.48% compared to head (9b97f87) 81.32%.
Report is 6 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #547      +/-   ##
==========================================
+ Coverage   79.48%   81.32%   +1.83%     
==========================================
  Files         145      234      +89     
  Lines        2710     3812    +1102     
  Branches      636      873     +237     
==========================================
+ Hits         2154     3100     +946     
- Misses        526      675     +149     
- Partials       30       37       +7     
Files Changed Coverage Δ
src/CourseAuthoringRoutes.jsx 100.00% <ø> (ø)
src/index.jsx 0.00% <ø> (ø)
src/store.js 100.00% <ø> (ø)
src/studio-header/Header.jsx 100.00% <ø> (ø)
src/generic/course-upload-image/data/api.js 25.00% <25.00%> (ø)
src/schedule-and-details/data/thunks.js 44.11% <44.11%> (ø)
src/generic/WysiwygEditor.jsx 51.72% <51.72%> (ø)
src/schedule-and-details/license-section/hooks.jsx 60.00% <60.00%> (ø)
src/schedule-and-details/utils.js 60.52% <60.52%> (ø)
src/schedule-and-details/hooks.jsx 64.00% <64.00%> (ø)
... and 66 more

... and 28 files with indirect coverage changes

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

@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch from 8ca4e48 to 611bdda Compare August 3, 2023 09:30
* fix: console errors on schedule and details page

* fix: schedule and details input border color

* fix: initial state for wysiwyg

fix: [2U-270] requirements section with disabled flags (#34)

feat: 2u-237 add minHeigh prop to component
@ruzniaievdm
ruzniaievdm force-pushed the ruzniaievdm/feat/schedule-and-details-mfe branch from 611bdda to 9554703 Compare August 3, 2023 09:47
@ruzniaievdm
ruzniaievdm marked this pull request as ready for review August 3, 2023 09:56
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 3, 2023
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @ruzniaievdm! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Depending on the page certain links should not show. For example, on the Schedule and details page the user should not see a link for the Schedule and details page

Comment on lines +47 to +51
<Hyperlink
destination={creativeCommonsAboutURL}
target="_blank"
showLaunchIcon={false}
>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Hyperlink
destination={creativeCommonsAboutURL}
target="_blank"
showLaunchIcon={false}
>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've removed it, but there should still be this link with comparing it to the legacy version

Comment thread src/schedule-and-details/license-section/license-selector/index.jsx Outdated
Comment thread src/generic/datepicker-control/DatepickerControl.jsx Outdated
Comment thread src/generic/datepicker-control/DatepickerControl.jsx
/>
))}
</ul>
<Button iconBefore={AddIcon} variant="outline-success" onClick={handleAdd}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Button iconBefore={AddIcon} variant="outline-success" onClick={handleAdd}>
<Button iconBefore={AddIcon} variant="primary" onClick={handleAdd}>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As i understand, we have already approved style design. Do we really need to apply these changes?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can skip these changes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Already added)

<ul className="learning-outcomes-list">
{learningInfo.map(renderLearningOutcomeItem)}
</ul>
<Button iconBefore={AddIcon} variant="outline-success" onClick={handleAdd}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Button iconBefore={AddIcon} variant="outline-success" onClick={handleAdd}>
<Button iconBefore={AddIcon} variant="primary" onClick={handleAdd}>

</Card.Body>
<Card.Divider />
<Card.Footer className="p-0 mt-2.5">
<Button variant="outline-danger" onClick={() => onDelete(idx)}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Button variant="outline-danger" onClick={() => onDelete(idx)}>
<Button variant="outline-primary" onClick={() => onDelete(idx)}>

onChange={(e) => onChange(e.target.value, 'introVideo')}
/>
<Button
variant="outline-danger"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
variant="outline-danger"
variant="outline-primary"

onChange={(e) => handleInputChange(e.target.value, idx)}
/>
<Button
variant="outline-danger"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
variant="outline-danger"
variant="outline-primary"

@KristinAoki

Copy link
Copy Markdown
Contributor

What was the reasoning behind using dropdown instead of form.control with the type equal to select? Mainly for curiosity, doesn't require any changes.

@KristinAoki

Copy link
Copy Markdown
Contributor

I also recommend merging in the latest release of frontend-lib-content-components to this PR as it a requirement for this PR.

@ruzniaievdm

Copy link
Copy Markdown
Contributor Author

@KristinAoki in my opinion we used Dropdown only because of the styling of the components, if we don't have a need for it, I can rewrite it to custom select

@ruzniaievdm

Copy link
Copy Markdown
Contributor Author

@KristinAoki referring to these changes we need to delete MFE urls.

After these changes utility getPagePath has been broken.

So can you explain me why during developing MFE pages we should remove process.env.ENABLE_SOME_PAGE_PAGES?

In this case where should the links to the pages in the sidebar to MFE or LEGACY lead to?

@ruzniaievdm
ruzniaievdm requested a review from KristinAoki August 7, 2023 11:37
@KristinAoki

Copy link
Copy Markdown
Contributor

@KristinAoki in my opinion we used Dropdown only because of the styling of the components, if we don't have a need for it, I can rewrite it to custom select

You can continue to use the Dropdown component. I was just curious since they both offer the same functionality and most forms use select.

@KristinAoki

Copy link
Copy Markdown
Contributor

@KristinAoki referring to these changes we need to delete MFE urls.

After these changes utility getPagePath has been broken.

So can you explain me why during developing MFE pages we should remove process.env.ENABLE_SOME_PAGE_PAGES?

In this case where should the links to the pages in the sidebar to MFE or LEGACY lead to?

Sorry for the confusion. When TNL was originally setting up the course authoring MFE we thought env variables would be the best way to control access to the new pages. However, we have changed our approach and are going to rely exclusively on Django waffle flags and page redirects. Now all the links should lead to the LEGACY page.

@ruzniaievdm

ruzniaievdm commented Aug 7, 2023

Copy link
Copy Markdown
Contributor Author

@KristinAoki thank you for your reply, I see your point, then in that case I remove the utility with the MFE/LEGACY selection and leave only references to LEGACY.

@mphilbrick211
mphilbrick211 requested a review from a team August 7, 2023 14:51
@KristinAoki
KristinAoki merged commit a0e37c0 into openedx:master Aug 8, 2023
@openedx-webhooks

Copy link
Copy Markdown

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

peterkulko pushed a commit to raccoongang/frontend-app-course-authoring that referenced this pull request Aug 21, 2023
wowkalucky pushed a commit to raccoongang/frontend-app-course-authoring that referenced this pull request Aug 22, 2023
* feat: configuration for xpert unit summaries (openedx#540)

Adds setting modal for Xpert unit summaries

Includes hiding the config section for xpert summary - 
this is done based on a flag from edx/ai-aside@3d113d2

* fix: load up-to-date config on studio fetch (openedx#548)

* refactor: added PropTypes declarations

* feat: add files and uploads page (openedx#541)

* fix: remove env variable for files and uploads (openedx#549)

* Update verbiage for Xpert Settings (openedx#550)

* chore: Update verbiage for Xpert configuration screen

* fix: Change "generate" to "display" in xpert modal text

* fix: Updated learn more link

* fix: Change link and add targets

* test: added test cases for hide discussion tab (openedx#552)

Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>

* feat: upgrade frontend-lib-content-components (openedx#554)

* feat: bump frontend-lib-content-components (openedx#559)

* fix: Change wording to not crowd xpert tile in preferences page (openedx#560)

* fix: width and height of asset preview (openedx#558)

* feat: flcc to 1.168.0 (openedx#561)

* feat: bump frontend-lib-content-components (openedx#562)

* fix: overflow-y scroll behavior (openedx#565)

* test: added test cases of discussion restriction (openedx#556)

* test: added test cases of discussion restriction

* refactor: added null default value for dataTestId

---------

Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>

* fix: Add enabled badge to xpert settings tile  (openedx#566)

* feat: Add "Enabled" badge to xpert settings tile

* fix: Update model with state instead of non-existent prop from response

* feat: bump frontend-lib-content-components (openedx#569)

* feat: Added Schedule and Details MFE page (openedx#547)

* refactor: some refactoring

---------

Co-authored-by: David Nuon <davidnuongm@gmail.com>
Co-authored-by: Zachary Hancock <zhancock@edx.org>
Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Co-authored-by: sundasnoreen12 <72802712+sundasnoreen12@users.noreply.github.com>
Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>
Co-authored-by: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com>
Co-authored-by: ruzniaievdm <ruzniaievdm@gmail.com>
wowkalucky pushed a commit to raccoongang/frontend-app-course-authoring that referenced this pull request Aug 23, 2023
* feat: moving new commits from upstream repository (#62)

* feat: configuration for xpert unit summaries (openedx#540)

Adds setting modal for Xpert unit summaries

Includes hiding the config section for xpert summary - 
this is done based on a flag from edx/ai-aside@3d113d2

* fix: load up-to-date config on studio fetch (openedx#548)

* refactor: added PropTypes declarations

* feat: add files and uploads page (openedx#541)

* fix: remove env variable for files and uploads (openedx#549)

* Update verbiage for Xpert Settings (openedx#550)

* chore: Update verbiage for Xpert configuration screen

* fix: Change "generate" to "display" in xpert modal text

* fix: Updated learn more link

* fix: Change link and add targets

* test: added test cases for hide discussion tab (openedx#552)

Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>

* feat: upgrade frontend-lib-content-components (openedx#554)

* feat: bump frontend-lib-content-components (openedx#559)

* fix: Change wording to not crowd xpert tile in preferences page (openedx#560)

* fix: width and height of asset preview (openedx#558)

* feat: flcc to 1.168.0 (openedx#561)

* feat: bump frontend-lib-content-components (openedx#562)

* fix: overflow-y scroll behavior (openedx#565)

* test: added test cases of discussion restriction (openedx#556)

* test: added test cases of discussion restriction

* refactor: added null default value for dataTestId

---------

Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>

* fix: Add enabled badge to xpert settings tile  (openedx#566)

* feat: Add "Enabled" badge to xpert settings tile

* fix: Update model with state instead of non-existent prop from response

* feat: bump frontend-lib-content-components (openedx#569)

* feat: Added Schedule and Details MFE page (openedx#547)

* refactor: some refactoring

---------

Co-authored-by: David Nuon <davidnuongm@gmail.com>
Co-authored-by: Zachary Hancock <zhancock@edx.org>
Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Co-authored-by: sundasnoreen12 <72802712+sundasnoreen12@users.noreply.github.com>
Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>
Co-authored-by: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com>
Co-authored-by: ruzniaievdm <ruzniaievdm@gmail.com>

* fix: Course updates - wrong datepicker value (#65)

* fix: [2u-631] fixed date checking

* fix: [2u-631] fixed validation schema

* fix: [2u-631] fixed indents

* feat: Course outline - Section highlights (#60)

* feat: [2u-336] add tests

* feat: [2u-271] fix button

* feat: [2u-336] add component, refactor header

* feat: [2u-342] add modal

* fix: [2u-342] fix translates and indents

* feat: [2u-342] add modal

* feat: [2u-342] add api

* feat: [2u-342] add tests and translates

* feat: [2u-342] fix indents

* fix: [2u-342] fix indents, variant and utils

* feat: [2u-342] fixed slice, thunks, hooks

* fix: [2u-342] fix translates

* fix: [2u-342] fix api

---------

Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local>

* feat: create organization filter on home page

---------

Co-authored-by: Peter Kulko <93188219+PKulkoRaccoonGang@users.noreply.github.com>
Co-authored-by: David Nuon <davidnuongm@gmail.com>
Co-authored-by: Zachary Hancock <zhancock@edx.org>
Co-authored-by: Kristin Aoki <42981026+KristinAoki@users.noreply.github.com>
Co-authored-by: sundasnoreen12 <72802712+sundasnoreen12@users.noreply.github.com>
Co-authored-by: SundasNoreen <sundas.noreen@arbisoft.com>
Co-authored-by: Raymond Zhou <56318341+rayzhou-bit@users.noreply.github.com>
Co-authored-by: vladislavkeblysh <138868841+vladislavkeblysh@users.noreply.github.com>
Co-authored-by: Vladislav Keblysh <vladislavkeblysh@Vladislavs-MacBook-Pro.local>
snglth pushed a commit to Abstract-Tech/community-theme-course-authoring that referenced this pull request Jan 9, 2024
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

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants