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.

Sign up for GitHub

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

Feature/CV2-5785: media dialog view ux updates to include media match reason #2256

Open
wants to merge 12 commits into
base: epic/CV2-5510-why-media-is-in-cluster
Choose a base branch
from
Open

Feature/CV2-5785: media dialog view ux updates to include media match reason #2256

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4afa1b8
adding MediaOriginBanner component to media page
danielevalverde Jan 20, 2025
1823f14
passing media_clust_relantioship props
danielevalverde Jan 20, 2025
1227945
refactoring and renaming props
danielevalverde Jan 20, 2025
ea3b53f
more code cleanup
danielevalverde Jan 21, 2025
8954e9d
remove component from sandbox
danielevalverde Jan 21, 2025
567950f
updating unit tests
danielevalverde Jan 21, 2025
0053975
Fix linter problems
danielevalverde Jan 21, 2025
da5c700
apply PR review
danielevalverde Jan 21, 2025
a579be2
remove more debugs
danielevalverde Jan 21, 2025
582c346
apply PR programing with Alex
danielevalverde Jan 21, 2025
ad7b79d
update tests
danielevalverde Jan 21, 2025
47bb428
add MediaOriginBanner to MediaComponent
danielevalverde Jan 21, 2025
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: 3 additions & 3 deletions localization/react-intl/src/app/components/media/MediaOrigin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
{
"id": "mediaOrigin.userAddedTooltip",
"description": "Tooltip message for User Added",
"defaultMessage": "<strong>{user}</strong> uploaded this media using the Check interface"
"defaultMessage": "<strong>{media_cluster_origin_user}</strong> uploaded this media using the Check interface"
},
{
"id": "mediaOrigin.userMerged",
Expand All @@ -27,7 +27,7 @@
{
"id": "mediaOrigin.userMergedTooltip",
"description": "Tooltip message for User Merged",
"defaultMessage": "<strong>{user}</strong> added this media by merging from another cluster"
"defaultMessage": "<strong>{media_cluster_origin_user}</strong> added this media by merging from another cluster"
},
{
"id": "mediaOrigin.userMatched",
Expand All @@ -37,7 +37,7 @@
{
"id": "mediaOrigin.userMatchedTooltip",
"description": "Tooltip message for User Matched",
"defaultMessage": "<strong>{user}</strong> accepted this media as a suggested match"
"defaultMessage": "<strong>{media_cluster_origin_user}</strong> accepted this media as a suggested match"
},
{
"id": "mediaOrigin.autoMatched",
Expand Down
14 changes: 7 additions & 7 deletions localization/react-intl/src/app/components/media/MediaOriginBanner.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,26 @@
{
"id": "mediaOriginBanner.tiplineSubmitted",
"description": "Message for Tipline Submitted",
"defaultMessage": "This media was submitted via <strong>Tipline</strong>"
"defaultMessage": "This media was submitted via <strong>Tipline</strong>,"
},
{
"id": "mediaOriginBanner.userAdded",
"description": "Message for User Matched",
"defaultMessage": "This media was added to the cluster by <strong>{user}</strong>"
"description": "Message for User Added",
"defaultMessage": "This media was added to the cluster by <strong>{media_cluster_origin_user}</strong>,"
},
{
"id": "mediaOriginBanner.userMerged",
"description": "Message for User Matched",
"defaultMessage": "This media was added to the cluster by <strong>{user}</strong> when merged from <strong><u>{cluster}</u></strong>"
"description": "Message for User Merged",
"defaultMessage": "This media was merged to the cluster by <strong>{media_cluster_origin_user}</strong>,"
},
{
"id": "mediaOriginBanner.userMatched",
"description": "Message for User Matched",
"defaultMessage": "This media was added to the cluster by <strong>{user}</strong> when accepted from <strong><u>{cluster}</u></strong>"
"defaultMessage": "This media was added to the cluster by <strong>{media_cluster_origin_confirmed_by}</strong> when accepted from <strong><u>{media_cluster_origin_title}</u></strong>,"
},
{
"id": "mediaOriginBanner.autoMatched",
"description": "Message for Auto Matched",
"defaultMessage": "This media was automatically matched to the cluster"
"defaultMessage": "This media was automatically matched to the cluster,"
}
]
26 changes: 0 additions & 26 deletions src/app/components/cds/Sandbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import TabWrapper from './menus-lists-dialogs/TabWrapper';
import Reorder from '../layout/Reorder';
import AddIcon from '../../icons/settings.svg';
import CalendarIcon from '../../icons/calendar_month.svg';
import MediaOriginBanner from '../media/MediaOriginBanner';
import ListIcon from '../../icons/list.svg';
import FigmaColorLogo from '../../icons/figma_color.svg';
import ArticleCard from '../search/SearchResultsCards/ArticleCard';
Expand All @@ -37,7 +36,6 @@ import ParsedText from '../ParsedText';
import ClusterCard from '../search/SearchResultsCards/ClusterCard';
import CheckFeedDataPoints from '../../CheckFeedDataPoints';
import { FlashMessageSetterContext } from '../FlashMessage';
import CheckMediaOrigin from '../../CheckMediaOrigin';
import styles from './sandbox.module.css';

const SandboxComponent = ({ admin }) => {
Expand Down Expand Up @@ -2389,30 +2387,6 @@ const SandboxComponent = ({ admin }) => {
</div>
</section>
}
{ (categoryTab === 'all' || categoryTab === 'media-cluster-origin-buttons') && (
<section>
<div className={styles.componentWrapper}>
<div className={cx('typography-subtitle2', [styles.componentName])}>
Media Origin Banner
<a
className={styles.figmaLink}
href="https://www.figma.com/design/aVRaTgms3H4jY8hOslFq5y/Media?node-id=901-1203&p=f&m=dev"
rel="noopener noreferrer"
target="_blank"
title="Figma Designs"
>
<FigmaColorLogo />
</a>

<MediaOriginBanner cluster="Foo bar" timestamp={1736876257} type={CheckMediaOrigin.TIPLINE_SUBMITTED} user="Smooch" />
<MediaOriginBanner cluster="Foo bar" timestamp={1736876257} type={CheckMediaOrigin.USER_ADDED} user="Bruce" />
<MediaOriginBanner cluster="Foo bar" timestamp={1736876257} type={CheckMediaOrigin.USER_MERGED} user="Kara" />
<MediaOriginBanner cluster="Bla" timestamp={1736876257} type={CheckMediaOrigin.USER_MATCHED} user="Clark" />
<MediaOriginBanner cluster="Foo bar" timestamp={1736876257} type={CheckMediaOrigin.AUTO_MATCHED} user="Alegre" />
</div>
</div>
</section>
)}
</div>
);
};
Expand Down
10 changes: 9 additions & 1 deletion src/app/components/cds/menus-lists-dialogs/MediaAndRequestsDialogComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,16 @@ import MediaRequests from '../../media/MediaRequests';
import { MediaCardLargeQueryRenderer } from '../../media/MediaCardLarge';
import FeedItemMediaDialog from '../../feed/FeedItemMediaDialog';
import dialogStyles from '../../../styles/css/dialog.module.css';
import MediaOriginBanner from '../../media/MediaOriginBanner';
import styles from './MediaAndRequestsDialog.module.css';

const MediaAndRequestsDialogComponent = ({
dialogTitle,
feedId,
media_cluster_origin,
media_cluster_origin_timestamp,
media_cluster_origin_user,
media_cluster_relationship,
mediaHeader,
mediaSlug,
onClick,
Expand Down Expand Up @@ -54,7 +59,10 @@ const MediaAndRequestsDialogComponent = ({
<div className={styles.columns}>
{ context === 'workspace' ?
<>
<MediaCardLargeQueryRenderer projectMediaId={projectMediaId} />
<div>
<MediaOriginBanner media_cluster_origin={media_cluster_origin} media_cluster_origin_timestamp={media_cluster_origin_timestamp} media_cluster_origin_user={media_cluster_origin_user} media_cluster_relationship={media_cluster_relationship} />
<MediaCardLargeQueryRenderer projectMediaId={projectMediaId} />
</div>
<div>
{ projectMediaId && projectMediaImportedId && ( // Show the toggle if we have two values to switch between
<div className={styles.toggle}>
Expand Down
4 changes: 2 additions & 2 deletions src/app/components/media/MediaCardLargeFooter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ const MediaCardLargeFooter = ({
),
(
<MediaOrigin
type={projectMedia.media_cluster_origin}
user={projectMedia.media_cluster_origin_user?.name}
media_cluster_origin={projectMedia.media_cluster_origin}
media_cluster_origin_user={projectMedia.media_cluster_origin_user?.name}
/>
),
]}
Expand Down
27 changes: 12 additions & 15 deletions src/app/components/media/MediaOrigin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import Tipline from '../../icons/question_answer.svg';
import Tooltip from '../cds/alerts-and-prompts/Tooltip';
import CheckMediaOrigin from '../../CheckMediaOrigin';

const getIconAndMessage = (type, user) => {
switch (type) {
const getIconAndMessage = (media_cluster_origin, media_cluster_origin_user) => {
switch (media_cluster_origin) {
case CheckMediaOrigin.TIPLINE_SUBMITTED:
return {
icon: <Tipline />,
Expand Down Expand Up @@ -42,10 +42,10 @@ const getIconAndMessage = (type, user) => {
),
tooltipMessage: (
<FormattedHTMLMessage
defaultMessage="<strong>{user}</strong> uploaded this media using the Check interface"
defaultMessage="<strong>{media_cluster_origin_user}</strong> uploaded this media using the Check interface"
Copy link
Contributor

Choose a reason for hiding this comment

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

I would change slightly from:
...using the Check interface
to
...using Check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

description="Tooltip message for User Added"
id="mediaOrigin.userAddedTooltip"
values={{ user }}
values={{ media_cluster_origin_user }}
/>
),
};
Expand All @@ -61,10 +61,10 @@ const getIconAndMessage = (type, user) => {
),
tooltipMessage: (
<FormattedHTMLMessage
defaultMessage="<strong>{user}</strong> added this media by merging from another cluster"
defaultMessage="<strong>{media_cluster_origin_user}</strong> added this media by merging from another cluster"
Copy link
Contributor

Choose a reason for hiding this comment

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

please change from
...merging from another cluster
to
...merging from another cluster of media

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

description="Tooltip message for User Merged"
id="mediaOrigin.userMergedTooltip"
values={{ user }}
values={{ media_cluster_origin_user }}
/>
),
};
Expand All @@ -80,10 +80,10 @@ const getIconAndMessage = (type, user) => {
),
tooltipMessage: (
<FormattedHTMLMessage
defaultMessage="<strong>{user}</strong> accepted this media as a suggested match"
defaultMessage="<strong>{media_cluster_origin_user}</strong> accepted this media as a suggested match"
description="Tooltip message for User Matched"
id="mediaOrigin.userMatchedTooltip"
values={{ user }}
values={{ media_cluster_origin_user }}
/>
),
};
Expand Down Expand Up @@ -114,8 +114,8 @@ const getIconAndMessage = (type, user) => {
}
};

const MediaOrigin = ({ type, user }) => {
const { icon, message, tooltipMessage } = getIconAndMessage(type, user);
const MediaOrigin = ({ media_cluster_origin, media_cluster_origin_user }) => {
const { icon, message, tooltipMessage } = getIconAndMessage(media_cluster_origin, media_cluster_origin_user);

return (
<Tooltip
Expand All @@ -135,13 +135,10 @@ const MediaOrigin = ({ type, user }) => {
);
};

MediaOrigin.defaultProps = {
user: 'Unknown User',
};

MediaOrigin.propTypes = {
type: PropTypes.number.isRequired,
user: PropTypes.string,
media_cluster_origin: PropTypes.number.isRequired,
media_cluster_origin_user: PropTypes.string.isRequired,
};

export default MediaOrigin;
12 changes: 6 additions & 6 deletions src/app/components/media/MediaOrigin.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,29 @@ import CheckMediaOrigin from '../../CheckMediaOrigin';

describe('<MediaOrigin />', () => {
it('should render a button with the correct icon and message for each type', () => {
const tiplineSubmitted = mountWithIntl(<MediaOrigin type={CheckMediaOrigin.TIPLINE_SUBMITTED} />);
const tiplineSubmitted = mountWithIntl(<MediaOrigin media_cluster_origin={CheckMediaOrigin.TIPLINE_SUBMITTED} media_cluster_origin_user="Smooch" />);
expect(tiplineSubmitted.find(Tipline).length).toEqual(1);
expect(tiplineSubmitted.html()).toMatch('Tipline Submitted');

const userAdded = mountWithIntl(<MediaOrigin type={CheckMediaOrigin.USER_ADDED} user="John Doe" />);
const userAdded = mountWithIntl(<MediaOrigin media_cluster_origin={CheckMediaOrigin.USER_ADDED} media_cluster_origin_user="John Doe" />);
expect(userAdded.find(PersonAdd).length).toEqual(1);
expect(userAdded.html()).toMatch('User Added');

const userMerged = mountWithIntl(<MediaOrigin type={CheckMediaOrigin.USER_MERGED} user="John Doe" />);
const userMerged = mountWithIntl(<MediaOrigin media_cluster_origin={CheckMediaOrigin.USER_MERGED} media_cluster_origin_user="John Doe" />);
expect(userMerged.find(Person).length).toEqual(1);
expect(userMerged.html()).toMatch('User Merged');

const userMatched = mountWithIntl(<MediaOrigin type={CheckMediaOrigin.USER_MATCHED} user="John Doe" />);
const userMatched = mountWithIntl(<MediaOrigin media_cluster_origin={CheckMediaOrigin.USER_MATCHED} media_cluster_origin_user="John Doe" />);
expect(userMatched.find(PersonCheck).length).toEqual(1);
expect(userMatched.html()).toMatch('User Matched');

const autoMatched = mountWithIntl(<MediaOrigin type={CheckMediaOrigin.AUTO_MATCHED} />);
const autoMatched = mountWithIntl(<MediaOrigin media_cluster_origin={CheckMediaOrigin.AUTO_MATCHED} media_cluster_origin_user="Alegre" />);
expect(autoMatched.find(Bolt).length).toEqual(1);
expect(autoMatched.html()).toMatch('Auto Matched');
});

it('should return null for invalid type', () => {
const wrapper = mountWithIntl(<MediaOrigin type={99} />);
const wrapper = mountWithIntl(<MediaOrigin media_cluster_origin={99} media_cluster_origin_user="user" />);
expect(wrapper.find('FormattedMessage').length).toEqual(0);
});
});
49 changes: 29 additions & 20 deletions src/app/components/media/MediaOriginBanner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,22 @@ import CheckMediaOrigin from '../../CheckMediaOrigin';
import { parseStringUnixTimestamp } from '../../helpers';
import TimeBefore from '../TimeBefore';

const getIconAndMessage = (type, user, cluster, timestamp) => {
const getIconAndMessage = (media_cluster_origin, media_cluster_relationship, media_cluster_origin_user, media_cluster_origin_timestamp) => {
// The default messages are temporary, and will be updated in the ticket CV2-5785
const formattedTimestamp = <TimeBefore date={parseStringUnixTimestamp(timestamp)} />;
switch (type) {
const formattedTimestamp = <TimeBefore date={parseStringUnixTimestamp(media_cluster_origin_timestamp)} />;
const media_cluster_origin_confirmed_by = media_cluster_relationship?.confirmed_by?.name;
const media_cluster_origin_title = media_cluster_relationship?.target?.title;

switch (media_cluster_origin) {
case CheckMediaOrigin.TIPLINE_SUBMITTED:
return {
icon: <Tipline />,
message: (
<>
<FormattedHTMLMessage
defaultMessage="This media was submitted via <strong>Tipline</strong> "
defaultMessage="This media was submitted via <strong>Tipline</strong>, "
description="Message for Tipline Submitted"
id="mediaOriginBanner.tiplineSubmitted"
values={{ timestamp }}
/>
{formattedTimestamp}
</>
Expand All @@ -36,10 +38,10 @@ const getIconAndMessage = (type, user, cluster, timestamp) => {
message: (
<>
<FormattedHTMLMessage
defaultMessage="This media was added to the cluster by <strong>{user}</strong> "
description="Message for User Matched"
defaultMessage="This media was added to the cluster by <strong>{media_cluster_origin_user}</strong>, "
description="Message for User Added"
id="mediaOriginBanner.userAdded"
values={{ user }}
values={{ media_cluster_origin_user }}
/>
{formattedTimestamp}
</>
Expand All @@ -51,10 +53,10 @@ const getIconAndMessage = (type, user, cluster, timestamp) => {
message: (
<>
<FormattedHTMLMessage
defaultMessage="This media was added to the cluster by <strong>{user}</strong> when merged from <strong><u>{cluster}</u></strong> "
description="Message for User Matched"
defaultMessage="This media was merged to the cluster by <strong>{media_cluster_origin_user}</strong>, "
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change from
This media was merged to the cluster by...
to
This media was merged into this cluster of media by...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

description="Message for User Merged"
id="mediaOriginBanner.userMerged"
values={{ user, cluster }}
values={{ media_cluster_origin_user }}
/>
{formattedTimestamp}
</>
Expand All @@ -66,10 +68,10 @@ const getIconAndMessage = (type, user, cluster, timestamp) => {
message: (
<>
<FormattedHTMLMessage
defaultMessage="This media was added to the cluster by <strong>{user}</strong> when accepted from <strong><u>{cluster}</u></strong> "
defaultMessage="This media was added to the cluster by <strong>{media_cluster_origin_confirmed_by}</strong> when accepted from <strong><u>{media_cluster_origin_title}</u></strong>, "
Copy link
Contributor

Choose a reason for hiding this comment

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

from
This media was added to the cluster by...
to
This media was added to the cluster of media by...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

description="Message for User Matched"
id="mediaOriginBanner.userMatched"
values={{ user, cluster }}
values={{ media_cluster_origin_confirmed_by, media_cluster_origin_title }}
/>
{formattedTimestamp}
</>
Expand All @@ -81,7 +83,7 @@ const getIconAndMessage = (type, user, cluster, timestamp) => {
message: (
<>
<FormattedHTMLMessage
defaultMessage="This media was automatically matched to the cluster "
defaultMessage="This media was automatically matched to the cluster, "
Copy link
Contributor

Choose a reason for hiding this comment

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

from
...matched to the cluster,...
to
...matched to the cluster of media,...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

description="Message for Auto Matched"
id="mediaOriginBanner.autoMatched"
/>
Expand All @@ -99,9 +101,9 @@ const getIconAndMessage = (type, user, cluster, timestamp) => {
};

const MediaOriginBanner = ({
cluster, timestamp, type, user,
media_cluster_origin, media_cluster_origin_timestamp, media_cluster_origin_user, media_cluster_relationship,
}) => {
const { icon, message } = getIconAndMessage(type, user, cluster, timestamp);
const { icon, message } = getIconAndMessage(media_cluster_origin, media_cluster_relationship, media_cluster_origin_user, media_cluster_origin_timestamp);

return (
<Alert
Expand All @@ -114,10 +116,17 @@ const MediaOriginBanner = ({
};

MediaOriginBanner.propTypes = {
cluster: PropTypes.string.isRequired,
timestamp: PropTypes.number.isRequired,
type: PropTypes.number.isRequired,
user: PropTypes.string.isRequired,
media_cluster_origin: PropTypes.number.isRequired,
media_cluster_origin_timestamp: PropTypes.number.isRequired,
media_cluster_origin_user: PropTypes.string.isRequired,
media_cluster_relationship: PropTypes.shape({
confirmed_by: PropTypes.shape({
name: PropTypes.string,
}),
target: PropTypes.shape({
title: PropTypes.string,
}),
}).isRequired,
};

export default MediaOriginBanner;
Loading
Loading