From b7968a49394236049f425c7ffa12ce53a096f5a5 Mon Sep 17 00:00:00 2001 From: Suejung Shin Date: Tue, 29 Oct 2024 15:06:16 -0700 Subject: [PATCH] feat: Add 'viewing as visitor' label (#3445) --- .../components/Navigator/Navigator.test.tsx | 59 ++++++++++++++++++- .../Header/components/Navigator/Navigator.tsx | 38 ++++++++++-- .../ListRepo/RepoTitleLink/RepoTitleLink.jsx | 39 ++++++++---- src/ui/ContextSwitcher/ContextSwitcher.jsx | 2 +- src/ui/Label/Label.stories.tsx | 14 ++++- src/ui/Label/Label.tsx | 9 ++- 6 files changed, 137 insertions(+), 24 deletions(-) diff --git a/src/layouts/Header/components/Navigator/Navigator.test.tsx b/src/layouts/Header/components/Navigator/Navigator.test.tsx index 1ad0157683..6225cb2a0c 100644 --- a/src/layouts/Header/components/Navigator/Navigator.test.tsx +++ b/src/layouts/Header/components/Navigator/Navigator.test.tsx @@ -133,6 +133,16 @@ const mockDetailOwner = { }, } +const mockDetailOwnerNotMyOrg = { + owner: { + ownerid: 1, + username: 'not-codecov', + avatarUrl: 'http://127.0.0.1/avatar-url', + isCurrentUserPartOfOrg: false, + isAdmin: false, + }, +} + const mockOwnerPageData = { owner: { username: 'codecov', @@ -174,6 +184,10 @@ describe('Header Navigator', () => { return HttpResponse.json({ data: mockMyContexts }) }), graphql.query('DetailOwner', (info) => { + if (!isMyOrg) { + return HttpResponse.json({ data: mockDetailOwnerNotMyOrg }) + } + return HttpResponse.json({ data: mockDetailOwner }) }), graphql.query('OwnerPageData', (info) => { @@ -203,6 +217,19 @@ describe('Header Navigator', () => { const repo = await screen.findByText('test-repo') expect(repo).toBeInTheDocument() }) + + it('should show Viewing as Visitor if appropriate', async () => { + setup({ isMyOrg: false }) + render(, { + wrapper: wrapper('/gh/not-codecov/test-repo'), + }) + + const org = await screen.findByText('not-codecov') + expect(org).toBeInTheDocument() + + const text = await screen.findByText('Viewing as visitor') + expect(text).toBeInTheDocument() + }) }) describe('when on self-hosted admin settings page', () => { @@ -222,7 +249,7 @@ describe('Header Navigator', () => { describe('when viewing owner page', () => { describe('and user is not part of the org', () => { - it('should render non-ContextSwitcher owner page variant', async () => { + it('should still render the user orgs dropdown', async () => { const { user } = setup({ isMyOrg: false }) render(, { wrapper: wrapper('/gh/not-codecov'), @@ -234,8 +261,23 @@ describe('Header Navigator', () => { await user.click(org) const sentryOrg = screen.queryByRole('link', { name: 'sentry' }) - expect(sentryOrg).not.toBeInTheDocument() + expect(sentryOrg).toBeInTheDocument() + }) + }) + + it('should show the fallback if not logged in', async () => { + const { user } = setup({ isMyOrg: false }) + render(, { + wrapper: wrapper('/gh/not-codecov'), }) + + const org = await screen.findByText('not-codecov') + expect(org).toBeInTheDocument() + + await user.click(org) + + const dropdownText = screen.queryByText('Install Codecov GitHub app') + expect(dropdownText).not.toBeInTheDocument() }) }) @@ -327,5 +369,18 @@ describe('Header Navigator', () => { expect(sentryOrg).toBeInTheDocument() expect(sentryOrg).toHaveAttribute('href', '/gh/sentry') }) + + it('should show Viewing as Visitor if appropriate', async () => { + setup({ isMyOrg: false }) + render(, { + wrapper: wrapper('/gh/not-codecov'), + }) + + const org = await screen.findByText('not-codecov') + expect(org).toBeInTheDocument() + + const text = await screen.findByText('Viewing as visitor') + expect(text).toBeInTheDocument() + }) }) }) diff --git a/src/layouts/Header/components/Navigator/Navigator.tsx b/src/layouts/Header/components/Navigator/Navigator.tsx index 703490215e..cb653fbe5f 100644 --- a/src/layouts/Header/components/Navigator/Navigator.tsx +++ b/src/layouts/Header/components/Navigator/Navigator.tsx @@ -5,6 +5,7 @@ import { useCrumbs } from 'pages/RepoPage/context' import { Me } from 'services/user' import Avatar from 'ui/Avatar' import Breadcrumb from 'ui/Breadcrumb' +import Label from 'ui/Label' import MyContextSwitcher from './MyContextSwitcher' @@ -18,9 +19,22 @@ function Navigator({ currentUser }: NavigatorProps) { const { path } = useRouteMatch() const { breadcrumbs } = useCrumbs() + const isCurrentUserPartOfOrg = ownerData?.isCurrentUserPartOfOrg + // Repo page if (path.startsWith('/:provider/:owner/:repo')) { - return + return ( +
+ + {' '} + + {isCurrentUserPartOfOrg === false ? ( + + ) : null} +
+ ) } // Self-hosted admin settings @@ -42,16 +56,21 @@ function Navigator({ currentUser }: NavigatorProps) { ) } - if (ownerData && !ownerData.isCurrentUserPartOfOrg) { + // Fallback instead of MyContextSwitcher if not logged in + if (!currentUser) { return (
-

{ownerData.username}

+

{ownerData?.username}

+ {isCurrentUserPartOfOrg === false ? ( + + ) : null}
) } - // Everything else uses MyContextSwitcher let pageName = 'owner' if (path.startsWith('/analytics/:provider/:owner')) { pageName = 'analytics' @@ -63,7 +82,16 @@ function Navigator({ currentUser }: NavigatorProps) { pageName = 'accountAdmin' } - return + return ( +
+ + {isCurrentUserPartOfOrg === false ? ( + + ) : null} +
+ ) } export default Navigator diff --git a/src/shared/ListRepo/RepoTitleLink/RepoTitleLink.jsx b/src/shared/ListRepo/RepoTitleLink/RepoTitleLink.jsx index a0dea1e215..e8f4b787da 100644 --- a/src/shared/ListRepo/RepoTitleLink/RepoTitleLink.jsx +++ b/src/shared/ListRepo/RepoTitleLink/RepoTitleLink.jsx @@ -2,14 +2,7 @@ import PropTypes from 'prop-types' import AppLink from 'shared/AppLink' import Icon from 'ui/Icon' - -function Badge({ children }) { - return ( - - {children} - - ) -} +import Label from 'ui/Label' const getRepoIconName = ({ activated, isRepoPrivate, active }) => !activated && active ? 'ban' : isRepoPrivate ? 'lock-closed' : 'globe-alt' @@ -36,8 +29,16 @@ function RepoTitleLink({ repo, showRepoOwner, pageName, disabledLink }) { {repo.name} - {isRepoPrivate && Private} - {active && !activated && Deactivated} + {isRepoPrivate && ( + + )} + {active && !activated && ( + + )} ) } @@ -59,9 +60,21 @@ function RepoTitleLink({ repo, showRepoOwner, pageName, disabledLink }) { {repo.name} - {isRepoPrivate && Private} - {active && !activated && Deactivated} - {repo?.isDemo && System generated} + {isRepoPrivate && ( + + )} + {active && !activated && ( + + )} + {repo?.isDemo && ( + + )} ) } diff --git a/src/ui/ContextSwitcher/ContextSwitcher.jsx b/src/ui/ContextSwitcher/ContextSwitcher.jsx index aeec990f70..e3818cd2d9 100644 --- a/src/ui/ContextSwitcher/ContextSwitcher.jsx +++ b/src/ui/ContextSwitcher/ContextSwitcher.jsx @@ -160,7 +160,7 @@ function ContextSwitcher({
    = { component: Label, argTypes: { variant: { - options: ['default', 'subtle'], + options: ['default', 'subtle', 'plain'], control: { type: 'select' }, }, }, @@ -43,6 +43,18 @@ export const SubtleLabel: Story = { ), } +export const PlainLabel: Story = { + args: { variant: 'plain' }, + render: (args) => ( +
    + +
    + +
    +
    + ), +} + export const LabelInherits: Story = { args: { variant: 'default', diff --git a/src/ui/Label/Label.tsx b/src/ui/Label/Label.tsx index 604d5655dc..24e57d3e39 100644 --- a/src/ui/Label/Label.tsx +++ b/src/ui/Label/Label.tsx @@ -9,6 +9,8 @@ const label = cva( variant: { default: 'border-current', subtle: 'border-ds-border-line text-ds-gray-senary bg-ds-gray-primary', + plain: + 'py-0.5 border-ds-gray-tertiary text-ds-gray-senary dark:bg-ds-gray-tertiary dark:text-ds-secondary-text', }, }, defaultVariants: { @@ -17,13 +19,16 @@ const label = cva( } ) -interface LabelProps extends VariantProps {} +interface LabelProps extends VariantProps { + className?: string +} const Label: React.FC> = ({ children, variant, + className, }) => { - return {children} + return {children} } export default Label