Skip to content

feat: [AXIMST-23] Course unit - Sidebar with unit info - #117

Merged
monteri merged 8 commits into
ts-developfrom
Peter_Kulko/sidebar-with-unit-info
Jan 22, 2024
Merged

monteri merged 8 commits into
ts-developfrom
Peter_Kulko/sidebar-with-unit-info

Conversation

@peterkulko

@peterkulko peterkulko commented Jan 17, 2024 •

Copy link
Copy Markdown

@peterkulko
peterkulko marked this pull request as draft January 17, 2024 22:49
@peterkulko peterkulko self-assigned this Jan 17, 2024
@peterkulko
peterkulko force-pushed the Peter_Kulko/sidebar-with-unit-info branch from d0ce47d to 2a69d9b Compare January 18, 2024 14:20
@peterkulko peterkulko changed the title fetat: [AXIMST-23] Course unit - Sidebar with unit info feat: [AXIMST-23] Course unit - Sidebar with unit info Jan 19, 2024
@peterkulko
peterkulko force-pushed the Peter_Kulko/sidebar-with-unit-info branch 4 times, most recently from 6dfd135 to 7bf91b8 Compare January 21, 2024 15:26
@codecov-commenter

codecov-commenter commented Jan 21, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (ts-develop@3812f03). Click here to learn what that means.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##             ts-develop     #117   +/-   ##
=============================================
  Coverage              ?   88.93%           
=============================================
  Files                 ?      558           
  Lines                 ?     9240           
  Branches              ?     1930           
=============================================
  Hits                  ?     8218           
  Misses                ?      974           
  Partials              ?       48           

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

@peterkulko
peterkulko marked this pull request as ready for review January 22, 2024 08:04
{intl.formatMessage(messages.actionButtonDiscardChangesTitle)}
</Button>
)}
{/* TODO: Unit copying functionality will be added to: https://youtrack.raccoongang.com/issue/AXIMST-375 */}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We will open this PR into upstream. I think it should not contain such comments that are related to our work to make sure PRs are merged faster without additional questions.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed

Comment thread src/course-unit/sidebar/hooks.jsx Outdated
import messages from './messages';
import { extractCourseUnitId } from './utils';

const useCourseUnitData = (unitData) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const useCourseUnitData = (unitData) => {
const useCourseUnitData = ({ hasChanges, published, visibilityState }) => {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed

Comment thread src/course-unit/sidebar/hooks.jsx Outdated
Comment on lines +13 to +34
let title = intl.formatMessage(messages.sidebarTitleDraftNeverPublished);
let releaseLabel = getUnitReleaseStatus(intl).release;

switch (visibilityState) {
case UNIT_VISIBILITY_STATES.staffOnly:
title = intl.formatMessage(messages.sidebarTitleVisibleToStaffOnly);
break;
case UNIT_VISIBILITY_STATES.live:
title = intl.formatMessage(messages.sidebarTitlePublishedAndLive);
releaseLabel = getUnitReleaseStatus(intl).released;
break;
case UNIT_VISIBILITY_STATES.ready:
releaseLabel = getUnitReleaseStatus(intl).scheduled;
break;
default:
if (published) {
title = hasChanges
? intl.formatMessage(messages.sidebarTitleDraftUnpublishedChanges)
: intl.formatMessage(messages.sidebarTitlePublishedNotYetReleased);
}
break;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let title = intl.formatMessage(messages.sidebarTitleDraftNeverPublished);
let releaseLabel = getUnitReleaseStatus(intl).release;
switch (visibilityState) {
case UNIT_VISIBILITY_STATES.staffOnly:
title = intl.formatMessage(messages.sidebarTitleVisibleToStaffOnly);
break;
case UNIT_VISIBILITY_STATES.live:
title = intl.formatMessage(messages.sidebarTitlePublishedAndLive);
releaseLabel = getUnitReleaseStatus(intl).released;
break;
case UNIT_VISIBILITY_STATES.ready:
releaseLabel = getUnitReleaseStatus(intl).scheduled;
break;
default:
if (published) {
title = hasChanges
? intl.formatMessage(messages.sidebarTitleDraftUnpublishedChanges)
: intl.formatMessage(messages.sidebarTitlePublishedNotYetReleased);
}
break;
}
const titleMessages = {
[UNIT_VISIBILITY_STATES.staffOnly]: messages.sidebarTitleVisibleToStaffOnly,
[UNIT_VISIBILITY_STATES.live]: messages.sidebarTitlePublishedAndLive,
default: published ? (hasChanges ? messages.sidebarTitleDraftUnpublishedChanges : messages.sidebarTitlePublishedNotYetReleased)
: messages.sidebarTitleDraftNeverPublished,
};
const releaseLabels = {
[UNIT_VISIBILITY_STATES.staffOnly]: releaseStatus.release,
[UNIT_VISIBILITY_STATES.live]: releaseStatus.released,
[UNIT_VISIBILITY_STATES.ready]: releaseStatus.scheduled,
default: releaseStatus.release,
};
const releaseStatus = getUnitReleaseStatus(intl);
const title = titleMessages[visibilityState] || titleMessages.default;
const releaseLabel = releaseLabels[visibilityState] || releaseLabels.default;

Generally try to use Object instead of switch-case

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added

Comment thread src/course-unit/sidebar/utils.jsx Outdated
Comment on lines +21 to +27
if (hasChanges && editedOn && editedBy) {
return intl.formatMessage(messages.publishInfoDraftSaved, { editedOn, editedBy });
} if (publishedOn && publishedBy) {
return intl.formatMessage(messages.publishLastPublished, { publishedOn, publishedBy });
}

return intl.formatMessage(messages.publishInfoPreviouslyPublished);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (hasChanges && editedOn && editedBy) {
return intl.formatMessage(messages.publishInfoDraftSaved, { editedOn, editedBy });
} if (publishedOn && publishedBy) {
return intl.formatMessage(messages.publishLastPublished, { publishedOn, publishedBy });
}
return intl.formatMessage(messages.publishInfoPreviouslyPublished);
if (hasChanges && editedOn && editedBy) {
return intl.formatMessage(messages.publishInfoDraftSaved, { editedOn, editedBy });
} else if (publishedOn && publishedBy) {
return intl.formatMessage(messages.publishLastPublished, { publishedOn, publishedBy });
} else {
return intl.formatMessage(messages.publishInfoPreviouslyPublished);
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added variable

Comment on lines +37 to +50
export const getReleaseInfo = (intl, releaseDate, releaseDateFrom) => {
if (releaseDate) {
return (
<span className="course-unit-sidebar-date-and-with">
<h6 className="course-unit-sidebar-date-timestamp m-0 d-inline">
{releaseDate}&nbsp;
</h6>
{intl.formatMessage(messages.releaseInfoWithSection, { sectionName: releaseDateFrom })}
</span>
);
}

return intl.formatMessage(messages.releaseInfoUnscheduled);
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export const getReleaseInfo = (intl, releaseDate, releaseDateFrom) => {
if (releaseDate) {
return (
<span className="course-unit-sidebar-date-and-with">
<h6 className="course-unit-sidebar-date-timestamp m-0 d-inline">
{releaseDate}&nbsp;
</h6>
{intl.formatMessage(messages.releaseInfoWithSection, { sectionName: releaseDateFrom })}
</span>
);
}
return intl.formatMessage(messages.releaseInfoUnscheduled);
};
export const getReleaseInfo = (intl, releaseDate, releaseDateFrom) => {
if (releaseDate) {
return {
isScheduled: true,
releaseDate,
releaseDateFrom,
sectionNameMessage: intl.formatMessage(messages.releaseInfoWithSection, { sectionName: releaseDateFrom }),
};
}
return {
isScheduled: false,
message: intl.formatMessage(messages.releaseInfoUnscheduled),
};
};

We don't use JSX in utils, please change the usage of this function

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

import React from 'react';
import { getReleaseInfo } from './utils';

const ReleaseInfoComponent = ({ intl, releaseDate, releaseDateFrom }) => {
  const releaseInfo = getReleaseInfo(intl, releaseDate, releaseDateFrom);

  if (releaseInfo.isScheduled) {
    return (
      <span className="course-unit-sidebar-date-and-with">
        <h6 className="course-unit-sidebar-date-timestamp m-0 d-inline">
          {releaseInfo.releaseDate}&nbsp;
        </h6>
        {releaseInfo.sectionNameMessage}
      </span>
    );
  }

  return <>{releaseInfo.message}</>;
};

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Created

Comment on lines +77 to +93
export const getIconVariant = (visibilityState, published, hasChanges) => {
if (visibilityState === UNIT_VISIBILITY_STATES.staffOnly) {
// Visible to staff only
return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
} if (visibilityState === UNIT_VISIBILITY_STATES.live) {
// Published and live
return { iconSrc: CheckCircleIcon, colorVariant: COLORS.GREEN };
} if (published && !hasChanges) {
// Published (not yet released)
return { iconSrc: CheckCircleOutlineIcon, colorVariant: COLORS.BLACK };
} if (published && hasChanges) {
// Draft (unpublished changes)
return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
}

return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
export const getIconVariant = (visibilityState, published, hasChanges) => {
if (visibilityState === UNIT_VISIBILITY_STATES.staffOnly) {
// Visible to staff only
return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
} if (visibilityState === UNIT_VISIBILITY_STATES.live) {
// Published and live
return { iconSrc: CheckCircleIcon, colorVariant: COLORS.GREEN };
} if (published && !hasChanges) {
// Published (not yet released)
return { iconSrc: CheckCircleOutlineIcon, colorVariant: COLORS.BLACK };
} if (published && hasChanges) {
// Draft (unpublished changes)
return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
}
return { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK };
};
export const getIconVariant = (visibilityState, published, hasChanges) => {
const iconVariants = {
[UNIT_VISIBILITY_STATES.staffOnly]: { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK },
[UNIT_VISIBILITY_STATES.live]: { iconSrc: CheckCircleIcon, colorVariant: COLORS.GREEN },
publishedNoChanges: { iconSrc: CheckCircleOutlineIcon, colorVariant: COLORS.BLACK },
publishedWithChanges: { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK },
default: { iconSrc: InfoOutlineIcon, colorVariant: COLORS.BLACK },
};
if (visibilityState in iconVariants) {
return iconVariants[visibilityState];
}
if (published) {
return hasChanges ? iconVariants.publishedWithChanges : iconVariants.publishedNoChanges;
}
return iconVariants.default;
};

If there is many-if logic it is preferable to try Object solution.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Changed

@peterkulko
peterkulko force-pushed the Peter_Kulko/sidebar-with-unit-info branch from c636ba3 to 08c268d Compare January 22, 2024 11:07
@peterkulko
peterkulko requested a review from monteri January 22, 2024 14:22
@monteri
monteri merged commit f1ec844 into ts-develop Jan 22, 2024
ihor-romaniuk pushed a commit that referenced this pull request Jan 25, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
ihor-romaniuk pushed a commit that referenced this pull request Feb 8, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
peterkulko added a commit that referenced this pull request Feb 13, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
ihor-romaniuk pushed a commit that referenced this pull request Feb 16, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
peterkulko added a commit that referenced this pull request Feb 27, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
peterkulko added a commit that referenced this pull request Feb 29, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
ihor-romaniuk pushed a commit that referenced this pull request Feb 29, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
ihor-romaniuk pushed a commit that referenced this pull request Feb 29, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
peterkulko added a commit that referenced this pull request Mar 10, 2024
* feat: added Sidebar with unit info

* feat: added unit location

* refactor: added legacy behavior

* feat: added live variant

* refactor: code refactoring

* feat: added tests and translations

* feat: added new font size

* refactor: after review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants