-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
ref(whats-new): Revamp "Whats New" #76818
Changes from 20 commits
d40550c
c546c6d
af17700
991aeae
3b892a0
70a8fda
01451b9
6fbc2f5
f6e4b7b
108efa8
65f0381
458fee5
7dbbef6
dd7a508
2f1422f
407d12d
deaa5c1
8e06a98
4442ed5
5a0586a
1c19edd
74cf7d8
3129e46
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
import {useCallback} from 'react'; | ||
import styled from '@emotion/styled'; | ||
|
||
import Badge from 'sentry/components/badge/badge'; | ||
import Tag from 'sentry/components/badge/tag'; | ||
import ExternalLink from 'sentry/components/links/externalLink'; | ||
import {t} from 'sentry/locale'; | ||
import {space} from 'sentry/styles/space'; | ||
import type {Broadcast} from 'sentry/types/system'; | ||
import {trackAnalytics} from 'sentry/utils/analytics'; | ||
import useOrganization from 'sentry/utils/useOrganization'; | ||
|
||
export const BROADCAST_CATEGORIES: Record<NonNullable<Broadcast['category']>, string> = { | ||
announcement: t('Announcement'), | ||
feature: t('New Feature'), | ||
blog: t('Blog Post'), | ||
event: t('Event'), | ||
video: t('Video'), | ||
}; | ||
|
||
interface BroadcastPanelItemProps | ||
extends Pick< | ||
Broadcast, | ||
'hasSeen' | 'category' | 'title' | 'message' | 'link' | 'mediaUrl' | ||
> {} | ||
|
||
export function BroadcastPanelItem({ | ||
hasSeen, | ||
title, | ||
message, | ||
link, | ||
mediaUrl, | ||
category, | ||
}: BroadcastPanelItemProps) { | ||
const organization = useOrganization(); | ||
|
||
const handlePanelClicked = useCallback(() => { | ||
trackAnalytics('whats_new.link_clicked', {organization, title, category}); | ||
}, [organization, title, category]); | ||
|
||
return ( | ||
<SidebarPanelItemRoot> | ||
<TextBlock> | ||
{category && | ||
(hasSeen ? ( | ||
<CategoryTag>{BROADCAST_CATEGORIES[category]}</CategoryTag> | ||
) : ( | ||
<CategoryBadge type="new">{BROADCAST_CATEGORIES[category]}</CategoryBadge> | ||
))} | ||
<Title hasSeen={hasSeen} href={link} onClick={handlePanelClicked}> | ||
{title} | ||
</Title> | ||
<Message>{message}</Message> | ||
</TextBlock> | ||
{mediaUrl && <Media src={mediaUrl} alt={title} />} | ||
</SidebarPanelItemRoot> | ||
); | ||
} | ||
|
||
const SidebarPanelItemRoot = styled('div')` | ||
line-height: 1.5; | ||
background: ${p => p.theme.background}; | ||
margin: 0 ${space(3)}; | ||
padding: ${space(2)} 0; | ||
|
||
:not(:first-child) { | ||
border-top: 1px solid ${p => p.theme.border}; | ||
} | ||
`; | ||
|
||
const Title = styled(ExternalLink)<Pick<BroadcastPanelItemProps, 'hasSeen'>>` | ||
font-size: ${p => p.theme.fontSizeLarge}; | ||
color: ${p => p.theme.blue400}; | ||
${p => !p.hasSeen && `font-weight: ${p.theme.fontWeightBold}`}; | ||
`; | ||
|
||
const Message = styled('div')` | ||
color: ${p => p.theme.subText}; | ||
`; | ||
|
||
const TextBlock = styled('div')` | ||
margin-bottom: ${space(1.5)}; | ||
display: flex; | ||
flex-direction: column; | ||
`; | ||
|
||
const Media = styled('img')` | ||
border-radius: ${p => p.theme.borderRadius}; | ||
border: 1px solid ${p => p.theme.translucentGray200}; | ||
max-width: 100%; | ||
`; | ||
|
||
const CategoryTag = styled(Tag)` | ||
margin-bottom: ${space(1)}; | ||
`; | ||
|
||
const CategoryBadge = styled(Badge)` | ||
margin-left: 0; | ||
margin-bottom: ${space(1)}; | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
import {BroadcastFixture} from 'sentry-fixture/broadcast'; | ||
import {OrganizationFixture} from 'sentry-fixture/organization'; | ||
|
||
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; | ||
|
||
import {BROADCAST_CATEGORIES} from 'sentry/components/sidebar/broadcastPanelItem'; | ||
import Broadcasts from 'sentry/components/sidebar/broadcasts'; | ||
import {SidebarPanelKey} from 'sentry/components/sidebar/types'; | ||
import type {Broadcast} from 'sentry/types/system'; | ||
import {trackAnalytics} from 'sentry/utils/analytics'; | ||
|
||
jest.mock('sentry/utils/analytics'); | ||
|
||
function renderMockRequests({ | ||
orgSlug, | ||
broadcastsResponse, | ||
}: { | ||
orgSlug: string; | ||
broadcastsResponse?: Broadcast[]; | ||
}) { | ||
MockApiClient.addMockResponse({ | ||
url: '/broadcasts/', | ||
method: 'PUT', | ||
}); | ||
MockApiClient.addMockResponse({ | ||
url: `/organizations/${orgSlug}/broadcasts/`, | ||
body: broadcastsResponse ?? [], | ||
}); | ||
} | ||
|
||
describe('Broadcasts', function () { | ||
const category = 'blog'; | ||
|
||
it('renders empty state', async function () { | ||
const organization = OrganizationFixture(); | ||
|
||
renderMockRequests({orgSlug: organization.slug}); | ||
|
||
render( | ||
<Broadcasts | ||
orientation="left" | ||
collapsed={false} | ||
currentPanel={SidebarPanelKey.BROADCASTS} | ||
onShowPanel={() => jest.fn()} | ||
hidePanel={jest.fn()} | ||
organization={organization} | ||
/> | ||
); | ||
|
||
expect(await screen.findByText(/No recent updates/)).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders a broadcast item with media content correctly', async function () { | ||
const organization = OrganizationFixture({features: ['what-is-new-revamp']}); | ||
const broadcast = BroadcastFixture({ | ||
mediaUrl: | ||
'https://images.ctfassets.net/em6l9zw4tzag/2vWdw7ZaApWxygugalbyOC/285525e5b7c9fbfa8fb814a69ab214cd/PerformancePageSketches_hero.jpg?w=2520&h=945&q=50&fm=webp', | ||
category, | ||
}); | ||
|
||
renderMockRequests({orgSlug: organization.slug, broadcastsResponse: [broadcast]}); | ||
|
||
render( | ||
<Broadcasts | ||
orientation="left" | ||
collapsed={false} | ||
currentPanel={SidebarPanelKey.BROADCASTS} | ||
onShowPanel={() => jest.fn()} | ||
hidePanel={jest.fn()} | ||
organization={organization} | ||
/> | ||
); | ||
|
||
// Verify that the broadcast content is rendered correctly | ||
expect(await screen.findByText(BROADCAST_CATEGORIES[category])).toBeInTheDocument(); | ||
const titleLink = screen.getByRole('link', {name: broadcast.title}); | ||
expect(titleLink).toHaveAttribute('href', broadcast.link); | ||
expect(screen.getByText(/Source maps are JSON/)).toBeInTheDocument(); | ||
|
||
// Simulate click and check if analytics tracking is called | ||
await userEvent.click(titleLink); | ||
expect(trackAnalytics).toHaveBeenCalledWith( | ||
'whats_new.link_clicked', | ||
expect.objectContaining({ | ||
title: broadcast.title, | ||
category, | ||
}) | ||
); | ||
}); | ||
|
||
it('renders deprecated broadcast experience', async function () { | ||
const organization = OrganizationFixture(); | ||
const broadcast = BroadcastFixture(); | ||
|
||
renderMockRequests({orgSlug: organization.slug, broadcastsResponse: [broadcast]}); | ||
|
||
render( | ||
<Broadcasts | ||
orientation="left" | ||
collapsed={false} | ||
currentPanel={SidebarPanelKey.BROADCASTS} | ||
onShowPanel={() => jest.fn()} | ||
hidePanel={jest.fn()} | ||
organization={organization} | ||
/> | ||
); | ||
|
||
expect(await screen.findByRole('link', {name: broadcast.cta})).toBeInTheDocument(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
import type {OnboardingTaskStatus} from 'sentry/types/onboarding'; | ||
import type {Organization} from 'sentry/types/organization'; | ||
|
||
export const isDone = (task: OnboardingTaskStatus) => | ||
task.status === 'complete' || task.status === 'skipped'; | ||
|
||
// To be passed as the `source` parameter in router navigation state | ||
// e.g. {pathname: '/issues/', state: {source: `sidebar`}} | ||
export const SIDEBAR_NAVIGATION_SOURCE = 'sidebar'; | ||
|
||
export function hasWhatIsNewRevampFeature(organization: Organization) { | ||
return organization.features.includes('what-is-new-revamp'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -228,17 +228,38 @@ export type PipelineInitialData = { | |
props: Record<string, any>; | ||
}; | ||
|
||
export type Broadcast = { | ||
cta: string; | ||
export interface Broadcast { | ||
dateCreated: string; | ||
dateExpires: string; | ||
/** | ||
* Has the item been seen? affects the styling of the panel item | ||
*/ | ||
hasSeen: boolean; | ||
id: string; | ||
isActive: boolean; | ||
/** | ||
* The URL to use for the CTA | ||
*/ | ||
link: string; | ||
/** | ||
* A message with muted styling which appears above the children content | ||
*/ | ||
message: string; | ||
title: string; | ||
}; | ||
/** | ||
* Category of the broadcast. | ||
* Synced with https://github.com/getsentry/sentry/blob/923a65508912c3e181e1c70cbdf076b7b956aa90/src/sentry/models/broadcast.py#L14 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we change the branch here to be master: https://github.com/getsentry/sentry/blob/master/src/sentry/models/broadcast.py There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
*/ | ||
category?: 'announcement' | 'feature' | 'blog' | 'event' | 'video'; | ||
/** | ||
* The text for the CTA link at the bottom of the panel item | ||
*/ | ||
cta?: string; | ||
/** | ||
* Image url | ||
*/ | ||
mediaUrl?: string; | ||
} | ||
|
||
// XXX(epurkhiser): The components list can be generated using jq | ||
// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two nits here - would prefer we call this
whats-new-revamp
if the flag hasn't been created yet. second, do we really need this util function ? i feel like since it is just a feature flag check we could have it beconst hasWhatsNewRevamp = organization.features.includes()...;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes initially I thought we would use it in more different places but it makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh and the feature flag has already been created