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: update next unit button plugin for left sidebar navigation usage #1578

Merged
merged 3 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/courseware/course/sequence/Sequence.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
import { useIntl } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';
import SequenceExamWrapper from '@edx/frontend-lib-special-exams';
import { useToggle } from '@openedx/paragon';

import PageLoading from '@src/generic/PageLoading';
import { useModel } from '@src/generic/model-store';
Expand All @@ -35,7 +34,6 @@ const Sequence = ({
previousSequenceHandler,
}) => {
const intl = useIntl();
const [isOpen, open, close] = useToggle();
const {
canAccessProctoredExams,
license,
Expand Down Expand Up @@ -145,6 +143,7 @@ const Sequence = ({

const renderUnitNavigation = (isAtTop) => (
<UnitNavigation
courseId={courseId}
sequenceId={sequenceId}
unitId={unitId}
isAtTop={isAtTop}
Expand Down Expand Up @@ -185,9 +184,6 @@ const Sequence = ({
{...{
nextSequenceHandler,
handleNavigate,
isOpen,
open,
close,
}}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { breakpoints, useWindowSize } from '@openedx/paragon';
import classNames from 'classnames';
import { useIntl } from '@edx/frontend-platform/i18n';
import { PluginSlot } from '@openedx/frontend-plugin-framework';
import { useSelector } from 'react-redux';

import { LOADED } from '@src/constants';
Expand All @@ -15,7 +13,7 @@ import { useModel } from '../../../../generic/model-store';

import messages from './messages';
import PreviousButton from './generic/PreviousButton';
import NextButton from './generic/NextButton';
import { NextUnitTopNavTriggerSlot } from '../../../../plugin-slots/NextUnitTopNavTriggerSlot';

const SequenceNavigation = ({
unitId,
Expand All @@ -24,11 +22,6 @@ const SequenceNavigation = ({
onNavigate,
nextHandler,
previousHandler,
nextSequenceHandler,
handleNavigate,
isOpen,
open,
close,
}) => {
const intl = useIntl();
const sequence = useModel('sequences', sequenceId);
Expand Down Expand Up @@ -83,36 +76,28 @@ const SequenceNavigation = ({
);

const renderNextButton = () => {
let buttonText;
const { exitActive, exitText } = GetCourseExitNavigation(courseId, intl);
const buttonText = (isLastUnit && exitText) ? exitText : intl.formatMessage(messages.nextButton);
const disabled = isLastUnit && !exitActive;

if (isLastUnit && exitText) {
buttonText = exitText;
} else if (!shouldDisplayNotificationTriggerInSequence) {
buttonText = intl.formatMessage(messages.nextButton);
}
return navigationDisabledNextSequence || (
<PluginSlot
id="next_button_slot"
pluginProps={{
<NextUnitTopNavTriggerSlot
{...{
courseId,
disabled,
buttonText: shouldDisplayNotificationTriggerInSequence ? null : buttonText,
buttonText,
nextLink,
sequenceId,
unitId,
nextSequenceHandler,
handleNavigate,
isOpen,
open,
close,
onClickHandler: nextHandler,
variant: 'link',
buttonStyle: 'next-btn',
}}
>
<NextButton
variant="link"
buttonStyle="next-btn"
onClick={nextHandler}
nextLink={nextLink}
disabled={disabled}
buttonLabel={shouldDisplayNotificationTriggerInSequence ? null : buttonText}
/>
</PluginSlot>
/>
);
};

Expand All @@ -132,21 +117,11 @@ SequenceNavigation.propTypes = {
onNavigate: PropTypes.func.isRequired,
nextHandler: PropTypes.func.isRequired,
previousHandler: PropTypes.func.isRequired,
close: PropTypes.func,
open: PropTypes.func,
isOpen: PropTypes.bool,
handleNavigate: PropTypes.func,
nextSequenceHandler: PropTypes.func,
};

SequenceNavigation.defaultProps = {
className: null,
unitId: null,
close: null,
open: null,
isOpen: false,
handleNavigate: null,
nextSequenceHandler: null,
};

export default SequenceNavigation;
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
import classNames from 'classnames';
import PropTypes from 'prop-types';
import { useIntl } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';

import { GetCourseExitNavigation } from '../../course-exit';

import { useSequenceNavigationMetadata } from './hooks';
import messages from './messages';
import PreviousButton from './generic/PreviousButton';
import NextButton from './generic/NextButton';
import { NextUnitTopNavTriggerSlot } from '../../../../plugin-slots/NextUnitTopNavTriggerSlot';

const UnitNavigation = ({
sequenceId,
unitId,
onClickPrevious,
onClickNext,
isAtTop,
courseId,
}) => {
const intl = useIntl();
const {
isFirstUnit, isLastUnit, nextLink, previousLink,
} = useSequenceNavigationMetadata(sequenceId, unitId);
const { courseId } = useSelector(state => state.courseware);

const renderPreviousButton = () => (
<PreviousButton
Expand All @@ -38,13 +38,33 @@ const UnitNavigation = ({
const { exitActive, exitText } = GetCourseExitNavigation(courseId, intl);
const buttonText = (isLastUnit && exitText) ? exitText : intl.formatMessage(messages.nextButton);
const disabled = isLastUnit && !exitActive;
const variant = 'outline-primary';
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Pulling these out into variables is a little weird to me. I prefer the text being included in the props. Is this a normal practice in this codebase?

Copy link
Member Author

Choose a reason for hiding this comment

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

I put them into a variable because they are being used in two spots and want to guarantee that their values always stay in sync

const buttonStyle = 'next-button justify-content-center';

if (isAtTop) {
return (
<NextUnitTopNavTriggerSlot
{...{
courseId,
variant,
buttonStyle,
buttonText,
disabled,
sequenceId,
nextLink,
onClickHandler: onClickNext,
}}
/>
);
}

return (
<NextButton
variant="outline-primary"
buttonStyle="next-button justify-content-center"
onClick={onClickNext}
variant={variant}
buttonStyle={buttonStyle}
onClickHandler={onClickNext}
disabled={disabled}
buttonLabel={buttonText}
buttonText={buttonText}
nextLink={nextLink}
hasEffortEstimate
/>
Expand All @@ -60,6 +80,7 @@ const UnitNavigation = ({
};

UnitNavigation.propTypes = {
courseId: PropTypes.string.isRequired,
sequenceId: PropTypes.string.isRequired,
unitId: PropTypes.string,
onClickPrevious: PropTypes.func.isRequired,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ describe('Unit Navigation', () => {
const store = await initializeTestStore({ courseMetadata, unitBlocks });
const { courseware } = store.getState();
mockData = {
courseId: courseware.courseId,
unitId: unitBlocks[1].id,
sequenceId: courseware.sequenceId,
onClickPrevious: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { isRtl, getLocale } from '@edx/frontend-platform/i18n';
import UnitNavigationEffortEstimate from '../UnitNavigationEffortEstimate';

const NextButton = ({
onClick,
buttonLabel,
onClickHandler,
buttonText,
nextLink,
variant,
buttonStyle,
Expand All @@ -20,16 +20,16 @@ const NextButton = ({
const navLink = pathname.startsWith('/preview') ? `/preview${nextLink}` : nextLink;
const buttonContent = hasEffortEstimate ? (
<UnitNavigationEffortEstimate>
{buttonLabel}
{buttonText}
</UnitNavigationEffortEstimate>
) : buttonLabel;
) : buttonText;

return (
<Button
variant={variant}
className={buttonStyle}
disabled={disabled}
onClick={onClick}
onClick={onClickHandler}
as={disabled ? undefined : Link}
to={disabled ? undefined : navLink}
iconAfter={nextArrow}
Expand All @@ -44,8 +44,8 @@ NextButton.defaultProps = {
};

NextButton.propTypes = {
onClick: PropTypes.func.isRequired,
buttonLabel: PropTypes.string.isRequired,
onClickHandler: PropTypes.func.isRequired,
buttonText: PropTypes.string.isRequired,
nextLink: PropTypes.string.isRequired,
variant: PropTypes.string.isRequired,
buttonStyle: PropTypes.string.isRequired,
Expand Down
55 changes: 55 additions & 0 deletions src/plugin-slots/NextUnitTopNavTriggerSlot/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Next Unit Top Navigation Trigger Slot

### Slot ID: `next_unit_top_nav_trigger_slot`

## Description

This slot is used to replace/modify/hide the next button used for unit and sequence navigation at the top of the unit page.

## Example

### Default content

**Next unit button in at top for left sidebar navigation**
![Screenshot of next unit button slot at the top for left sidebar navigation with default content](./screenshot_unit_at_top_default.png)

**Next unit button in horizontal navigation**
![Screenshot of horizontal navigaton next unit button slot with default content](./screenshot_horizontal_nav_default.png)

### Replaced with custom component

**Next unit button in at top for left sidebar navigation**
![Screenshot of 📊 in next unit slot at the top for left sidebar navigation](./screenshot_unit_at_top_custom.png)

**Next unit button in horizontal navigation**
![Screenshot of 📊 in next unit slot for horizontal navigaton](./screenshot_horizontal_nav_default.png)
![📊 in next unit slot](./screenshot_horizontal_nav_custom.png)

The following `env.config.jsx` will replace the next unit/sequence button at the top of the unit page. This can be used control the
button's `onClick` atrribute or change the name of the button.

```js
import { DIRECT_PLUGIN, PLUGIN_OPERATIONS } from '@openedx/frontend-plugin-framework';

const config = {
pluginSlots: {
next_unit_top_nav_trigger_slot: {
keepDefault: false,
plugins: [
{
op: PLUGIN_OPERATIONS.Insert,
widget: {
id: 'custom_button_component',
type: DIRECT_PLUGIN,
RenderWidget: () => (
<button>📊</button>
),
},
},
]
}
},
}

export default config;
```
53 changes: 53 additions & 0 deletions src/plugin-slots/NextUnitTopNavTriggerSlot/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import React from 'react';

import { PluginSlot } from '@openedx/frontend-plugin-framework';

import NextButton from '../../courseware/course/sequence/sequence-navigation/generic/NextButton';

interface Props {
courseId: string | '';
disabled: boolean;
buttonText: string | '';
nextLink: string;
sequenceId: string;
unitId: string;
onClickHandler: () => void;
variant: string;
buttonStyle: string;
}

export const NextUnitTopNavTriggerSlot : React.FC<Props> = ({
courseId,
disabled,
buttonText,
nextLink,
sequenceId,
onClickHandler,
variant,
buttonStyle,
}) => (
<PluginSlot
id="next_unit_top_nav_trigger_slot"
pluginProps={{
courseId,
disabled,
buttonText,
nextLink,
sequenceId,
onClickHandler,
variant,
buttonStyle,
}}
>
<NextButton
{...{
variant,
buttonStyle,
onClickHandler,
nextLink,
disabled,
buttonText,
}}
/>
</PluginSlot>
);
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading