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

Appless non-PAT org dropdown clarity changes #3628

Open
wants to merge 2 commits into
base: cy/non_pat_appless
Choose a base branch
from
Open

Appless non-PAT org dropdown clarity changes #3628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b0df6e
Appless non-PAT org dropdown clarity changes
calvin-codecov Jan 3, 2025
36c4e50
test: Add personal organization test
calvin-codecov Jan 8, 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
13 changes: 10 additions & 3 deletions src/ui/ContextSwitcher/ContextSwitcher.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ describe('ContextSwitcher', () => {
]}
currentUser={{
defaultOrgUsername: 'spotify',
username: 'laudna',
}}
src="imageUrl"
isLoading={false}
Expand All @@ -232,7 +233,9 @@ describe('ContextSwitcher', () => {
)
await user.click(button)

const laudnaUsers = await screen.findAllByText('laudna')
const laudnaUsers = await screen.findAllByText(
"laudna's personal organization"
)
expect(laudnaUsers.length).toBe(2)

const codecovOwner = await screen.findByText('codecov')
Expand Down Expand Up @@ -290,7 +293,9 @@ describe('ContextSwitcher', () => {
{ wrapper: wrapper() }
)

const installCopy = await screen.findByText(/Install Codecov GitHub app/)
const installCopy = await screen.findByText(
/To add another organization, install Codecov GitHub App/
)
expect(installCopy).toBeInTheDocument()
expect(installCopy).toHaveAttribute(
'href',
Expand Down Expand Up @@ -761,7 +766,9 @@ describe('ContextSwitcher', () => {
}
)

const installCopy = await screen.findByText(/Install Codecov GitHub app/)
const installCopy = await screen.findByText(
/To add another organization, install Codecov GitHub App/
)
expect(installCopy).toBeInTheDocument()
expect(installCopy).toHaveAttribute(
'href',
Expand Down
20 changes: 17 additions & 3 deletions src/ui/ContextSwitcher/ContextSwitcher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ interface ContextItemProps {
owner: { username: string | null } | null
pageName: string
}
currentUserUsername: string | null
defaultOrgUsername: string | null
setToggle: (arg: boolean) => void
owner?: string
}

function ContextItem({
context,
currentUserUsername,
defaultOrgUsername,
setToggle,
owner,
Expand Down Expand Up @@ -77,7 +79,9 @@ function ContextItem({
>
<Avatar user={contextOwner} />
<div className={cs('mx-1', { 'font-semibold': owner === orgUsername })}>
{orgUsername}
{!!orgUsername && orgUsername === currentUserUsername
? `${orgUsername}'s personal organization`
: orgUsername || ''}
</div>
</Button>
</li>
Expand Down Expand Up @@ -142,6 +146,8 @@ export interface Props {
contexts: Context[]
currentUser: {
defaultOrgUsername: string | null
username: string | null
avatarUrl: string
}
activeContext: {
avatarUrl: string
Expand Down Expand Up @@ -169,6 +175,7 @@ function ContextSwitcher({
const wrapperRef = useCloseOnLooseFocus({ setToggle })
const intersectionRef = useLoadMore({ onLoadMore })
const defaultOrgUsername = currentUser?.defaultOrgUsername
const currentUserUsername = currentUser?.username

const isGh = providerToName(provider) === 'Github'
const isSelfHosted = config.IS_SELF_HOSTED
Expand All @@ -177,6 +184,7 @@ function ContextSwitcher({
// self-hosted cannot use default "codecov" app (must set up custom one)
const shouldShowGitHubInstallLink =
isGh && (isSelfHosted ? isCustomGitHubApp : true)
const displayUsername = activeContext?.username ?? owner

return (
<div id="context-switcher" className="relative text-sm" ref={wrapperRef}>
Expand All @@ -193,7 +201,12 @@ function ContextSwitcher({
onClick={() => setToggle((toggle) => !toggle)}
>
<Avatar user={activeContext} />
<p className="ml-1">{activeContext?.username ?? owner}</p>
<p className="ml-1">
{displayUsername}
{displayUsername === currentUserUsername
? "'s personal organization"
: ''}
</p>
Comment on lines +204 to +209
Copy link
Contributor

Choose a reason for hiding this comment

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

I was playing around with this a little, because the current styles feel a bit much for all this text:

Screenshot 2025-01-09 at 12 02 10

I came up with two tiny alternatives that are fairly quick changes, the first removing the boldness and making it a little smaller:

Screenshot 2025-01-09 at 12 01 03

And the second just removing the boldness:

Screenshot 2025-01-09 at 12 01 16

Let me know what you think of these!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the last one! Would that work for, @Adal3n3 ?

Copy link

Choose a reason for hiding this comment

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

@nicholas-codecov @calvin-codecov Thanks for showing different variation of the "personal org" font style. After playing around the actual one in this pr, I prefer the first option as it looks consistent and smooth where second and third look a bit busy. If that's ok, please use the first option.

<span
aria-hidden="true"
className={cs('transition-transform', {
Expand Down Expand Up @@ -221,12 +234,13 @@ function ContextSwitcher({
hook="context-switcher-gh-install-link"
>
<Icon name="plusCircle" />
Install Codecov GitHub app
To add another organization, install Codecov GitHub App
</A>
</li>
) : null}
{contexts.map((context) => (
<ContextItem
currentUserUsername={currentUserUsername}
defaultOrgUsername={defaultOrgUsername}
context={context}
key={context?.owner?.username}
Expand Down
Loading