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.

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

[PAY-2727] Add images to oauth popup #8216

Merged
merged 1 commit into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 3 additions & 21 deletions packages/web/src/pages/oauth-login-page/OAuthLoginPage.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,6 @@
margin-top: var(--harmony-unit-8);
}

.infoSectionTitle {
color: var(--harmony-n-700);
line-height: 100%;
font-size: var(--harmony-font-m);
font-weight: var(--harmony-font-bold);
text-transform: uppercase;
letter-spacing: 0.5px;
margin-bottom: var(--harmony-unit-4);
}

.container {
width: 375px;
flex-direction: column;
Expand All @@ -74,10 +64,11 @@
}

.tile {
margin-top: var(--harmony-unit-2);
background-color: var(--harmony-n-25);
border: 1px solid var(--harmony-n-100);
border-radius: 8px;
padding: var(--harmony-unit-4) var(--harmony-unit-3);
padding: var(--harmony-unit-3) var(--harmony-unit-3);
}

.permissionContainer {
Expand Down Expand Up @@ -109,15 +100,6 @@
border-top: solid 1px var(--harmony-n-100);
}

.permissionTextContainer {
overflow: auto;
margin-left: var(--harmony-unit-4);
}

.permissionDetailTextContainer {
margin-top: var(--harmony-unit-2);
}

.permissionText {
color: var(--harmony-neutral);
font-weight: var(--harmony-font-medium);
Expand Down Expand Up @@ -182,7 +164,7 @@
/* Add padding bottom because Safari has issue where it clips off the bottom of the text otherwise */
padding-bottom: 2px;
background: none;
color: var(--harmony-secondary);
color: var(--harmony-primary);
font-size: var(--harmony-font-s);
font-weight: var(--harmony-font-medium);
line-height: 100%;
Expand Down
70 changes: 54 additions & 16 deletions packages/web/src/pages/oauth-login-page/OAuthLoginPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ import { FormEvent, useLayoutEffect, useState } from 'react'

import { Name, ErrorLevel } from '@audius/common/models'
import { accountSelectors, signOutActions } from '@audius/common/store'
import { IconValidationX } from '@audius/harmony'
import {
Flex,
IconEmbed,
IconTransaction,
IconValidationX,
Text,
TextLink
} from '@audius/harmony'
import cn from 'classnames'
import { useDispatch, useSelector } from 'react-redux'

import AppIcon from 'assets/img/appIcon.png'
import { make, useRecord } from 'common/store/analytics/actions'
import Input from 'components/data-entry/Input'
import LoadingSpinner from 'components/loading-spinner/LoadingSpinner'
Expand Down Expand Up @@ -145,6 +153,7 @@ export const OAuthLoginPage = () => {
userAlreadyWriteAuthorized,
apiKey,
appName,
appImage,
userEmail,
authorize,
txParams
Expand Down Expand Up @@ -236,15 +245,6 @@ export const OAuthLoginPage = () => {
const isSubmitDisabled =
generalSubmitError === messages.disconnectDashboardWalletWrongUserError

let titleText
if (!isLoggedIn) {
titleText = messages.signInAndAuthorizePrompt(appName as string)
} else if (userAlreadyWriteAuthorized) {
titleText = messages.alreadyAuthorizedContinuePrompt(appName as string)
} else {
titleText = messages.alreadyLoggedInAuthorizePrompt(appName as string)
}

if (queryParamsError) {
return (
<ContentWrapper>
Expand Down Expand Up @@ -272,9 +272,36 @@ export const OAuthLoginPage = () => {

return (
<ContentWrapper>
<div className={cn(styles.centeredContent, styles.titleContainer)}>
<h1 className={styles.title}>{titleText}</h1>
</div>
<Flex alignItems='center' direction='column'>
<Flex gap='l' alignItems='center' mb='l'>
<Flex h='88px' w='88px'>
<img src={AppIcon} alt={'Audius Logo'} />
</Flex>
<IconTransaction color='default' />
<Flex h='88px' w='88px'>
{appImage ? (
<img src={appImage} alt={`${appName} Image`} />
) : (
<Flex
w='100%'
justifyContent='center'
alignItems='center'
borderRadius='l'
css={{ backgroundColor: 'var(--harmony-n-200)' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is what Paper is for? Probably shouldn't be setting background colors like this.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's a again a weird icon component rather than a dialog or pop up and the background color is outside of our semantic values. will merge this but chat w julian some

>
<IconEmbed
color='subdued'
css={{ width: '48px', height: '48px' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sure there's probably a harmony constant that should be used here. But I'm not certain what it is 😅

/>
</Flex>
)}
</Flex>
</Flex>
<Text variant='body' size='l'>{`${messages.allow}:`}</Text>
<Text variant='heading' size='s'>
{appName}
</Text>
</Flex>
{userAlreadyWriteAuthorized ? null : (
<PermissionsSection
scope={scope}
Expand All @@ -287,7 +314,13 @@ export const OAuthLoginPage = () => {
<div className={styles.formArea}>
{isLoggedIn ? (
<div className={styles.userInfoContainer}>
<h3 className={styles.infoSectionTitle}>{messages.signedInAs}</h3>
<Text
variant='body'
size='m'
css={{ color: 'var(--harmony-n-600)' }}
>
{messages.signedInAs}
</Text>
<div className={styles.tile}>
<ProfileInfo
displayNameClassName={styles.userInfoDisplayName}
Expand All @@ -299,9 +332,14 @@ export const OAuthLoginPage = () => {
/>
</div>
<div className={styles.signOutButtonContainer}>
<button className={styles.linkButton} onClick={handleSignOut}>
<TextLink
variant='visible'
textVariant='body'
size='s'
onClick={handleSignOut}
>
{messages.signOut}
</button>
</TextLink>
</div>
<CTAButton
isLoading={isSubmitting}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
import { ReactNode } from 'react'

import { IconAudiusLogoHorizontalColor } from '@audius/harmony'

import styles from '../OAuthLoginPage.module.css'

export const ContentWrapper = ({ children }: { children: ReactNode }) => (
<div className={styles.wrapper}>
<div className={styles.container}>
<div className={styles.centeredContent}>
<div className={styles.logoContainer}>
<IconAudiusLogoHorizontalColor width='194' height='auto' />
</div>
</div>
{children}
</div>
<div className={styles.container}>{children}</div>
</div>
)
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { PropsWithChildren } from 'react'

import {
Flex,
IconEmailAddress,
IconPencil,
IconVisibilityPublic
IconVisibilityPublic,
Text
} from '@audius/harmony'
import cn from 'classnames'

Expand All @@ -15,15 +17,15 @@ import { WriteOnceParams, WriteOnceTx } from '../utils'

type PermissionTextProps = PropsWithChildren<{}>
const PermissionText = ({ children }: PermissionTextProps) => {
return <span className={styles.permissionText}>{children}</span>
return <div className={styles.permissionText}>{children}</div>
}

type PermissionDetailProps = PropsWithChildren<{
className?: string
}>
const PermissionDetail = ({ className, children }: PermissionDetailProps) => {
return (
<div className={cn(styles.permissionDetailTextContainer)}>
<div>
<span
className={cn(
styles.permissionText,
Expand Down Expand Up @@ -62,9 +64,9 @@ export const PermissionsSection = ({
return (
<>
<div className={styles.permsTitleContainer}>
<h3 className={styles.infoSectionTitle}>
<Text variant='body' size='m' css={{ color: 'var(--harmony-n-600)' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should be assigned via the color prop and use a semantic color

Copy link
Member Author

Choose a reason for hiding this comment

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

same that this is not semantic per the designs

{messages.permissionsRequestedHeader}
</h3>
</Text>
</div>
<div className={styles.tile}>
<div className={styles.permissionContainer}>
Expand All @@ -85,27 +87,34 @@ export const PermissionsSection = ({
)}
</div>

<div className={styles.permissionTextContainer}>
<PermissionText>
<Flex ml='l' gap='s' direction='column'>
<Text variant='body' size='m'>
{scope === 'write'
? messages.writeAccountAccess
: scope === 'write_once'
? getWriteOncePermissionTitle(tx)
: messages.readOnlyAccountAccess}
</PermissionText>
</Text>
{scope === 'write' ? (
<PermissionDetail>
{messages.doesNotGrantAccessTo}
<br />
{messages.walletsOrDMs}
</PermissionDetail>
<PermissionText>
<Text variant='body' size='s' color='subdued'>
{messages.writeAccessGrants}
</Text>
</PermissionText>
) : null}
{scope === 'write_once' ? (
<PermissionDetail>
{txParams?.wallet.slice(0, 6)}...{txParams?.wallet.slice(-4)}
</PermissionDetail>
) : null}
</div>
{scope === 'read' ? (
<PermissionText>
<Text variant='body' size='s' color='subdued'>
{messages.readOnlyGrants}
</Text>
</PermissionText>
) : null}
</Flex>
</div>
<div
className={cn(
Expand All @@ -121,7 +130,7 @@ export const PermissionsSection = ({
className={cn(styles.atSignIcon)}
/>
</div>
<div className={styles.permissionTextContainer}>
<Flex ml='l' gap='s' direction='column'>
<PermissionText>{messages.emailAddressAccess}</PermissionText>
{isLoggedIn ? (
<PermissionDetail
Expand All @@ -141,7 +150,7 @@ export const PermissionsSection = ({
)}
</PermissionDetail>
) : null}
</div>
</Flex>
</div>
</div>
</>
Expand Down
3 changes: 3 additions & 0 deletions packages/web/src/pages/oauth-login-page/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export const useOAuthSetup = ({
const [registeredDeveloperAppName, setRegisteredDeveloperAppName] =
useState<string>()
const appName = registeredDeveloperAppName ?? queryParamAppName
const [appImage, setAppImage] = useState<string>()

const [userAlreadyWriteAuthorized, setUserAlreadyWriteAuthorized] =
useState<boolean>()
Expand Down Expand Up @@ -250,6 +251,7 @@ export const useOAuthSetup = ({
return
}
setRegisteredDeveloperAppName(developerApp.name)
setAppImage(developerApp.imageUrl)
}
fetchDeveloperAppName()
}, [apiKey, queryParamAppName, queryParamsError, scope])
Expand Down Expand Up @@ -512,6 +514,7 @@ export const useOAuthSetup = ({
userAlreadyWriteAuthorized,
apiKey,
appName,
appImage,
userEmail,
authorize,
tx,
Expand Down
18 changes: 8 additions & 10 deletions packages/web/src/pages/oauth-login-page/messages.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
export const messages = {
alreadyLoggedInAuthorizePrompt: (appName: string) =>
`Authorize ${appName} to use your Audius account?`,
signInAndAuthorizePrompt: (appName: string) =>
`Sign in to allow ${appName} to use your Audius account?`,
alreadyAuthorizedContinuePrompt: (appName: string) => `Sign in to ${appName}`,
allow: 'Allow Audius to Connect to',
permissionsRequestedHeader: 'This application will receive',
readOnlyAccountAccess: 'Read-only access to your account',
readOnlyAccountAccess: 'Read-only Access',
readOnlyGrants:
'This app cannot interact with or make changes to your account.',
connectDashboardWalletAccess:
'Permission to link this wallet to your account',
disconnectDashboardWalletAccess:
'Permission to unlink this wallet from your account',
writeAccountAccess: 'Read/Write access to your account',
writeAccountAccess: 'Read/Write Access',
emailLoading: 'Email loading',
emailAddressAccess: 'Your Email Address',
doesNotGrantAccessTo: 'Does not grant access to:',
walletsOrDMs: 'Wallets or Direct Messages',
signOut: 'Not you? Sign Out & Switch Account',
writeAccessGrants:
'Grant this app permission to make changes to your account on your behalf.',
signOut: 'Sign Out',
signUp: `Don't have an account? Sign up`,
authorizeButton: 'Authorize App',
continueButton: 'Continue',
Expand Down