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

feat: firefox extension permission ui #3864

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
30 changes: 29 additions & 1 deletion packages/extension/src/newtab/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,20 @@ import { DndContextProvider } from '@dailydotdev/shared/src/contexts/DndContext'
import usePersistentState from '@dailydotdev/shared/src/hooks/usePersistentState';
import { LazyModal } from '@dailydotdev/shared/src/components/modals/common/types';
import { structuredCloneJsonPolyfill } from '@dailydotdev/shared/src/lib/structuredClone';
import usePersistentContext from '@dailydotdev/shared/src/hooks/usePersistentContext';
import { checkIsFirefox } from '@dailydotdev/shared/src/lib/func';
import { ExtensionContextProvider } from '../contexts/ExtensionContext';
import CustomRouter from '../lib/CustomRouter';
import { version } from '../../package.json';
import MainFeedPage from './MainFeedPage';
import { BootDataProvider } from '../../../shared/src/contexts/BootProvider';
import { getContentScriptPermissionAndRegister } from '../lib/extensionScripts';
import { useContentScriptStatus } from '../../../shared/src/hooks';
import {
FirefoxPermission,
FirefoxPermissionType,
} from '../permission/FirefoxPermission';
import { FirefoxPermissionDeclined } from '../permission/FirefoxPermissionDeclined';

structuredCloneJsonPolyfill();

Expand All @@ -67,7 +74,6 @@ Modal.defaultStyles = {};
const getRedirectUri = () => browser.runtime.getURL('index.html');
function InternalApp(): ReactElement {
useError();
useLazyModal();
sshanzel marked this conversation as resolved.
Show resolved Hide resolved
useWebVitals();
const { openModal } = useLazyModal();
const { setCurrentPage, currentPage, promptUninstallExtension } =
Expand All @@ -94,6 +100,11 @@ function InternalApp(): ReactElement {
}
}, [analyticsConsent, analyticsConsentPrompt]);

const [firefoxPermission, setFirefoxPermission, isFetched] =
usePersistentContext<FirefoxPermissionType | null>(
'firefox_accepted_permissions',
null,
);
const { unreadCount } = useNotificationContext();
const { closeLogin, shouldShowLogin, loginState } = useContext(AuthContext);
const { contentScriptGranted } = useContentScriptStatus();
Expand All @@ -106,6 +117,7 @@ function InternalApp(): ReactElement {
const isPageReady =
(growthbook?.ready && router?.isReady && isAuthReady) || isTesting;
const shouldRedirectOnboarding = !user && isPageReady && !isTesting;
const isFirefox = checkIsFirefox();

useEffect(() => {
if (routeChangedCallbackRef.current && isPageReady) {
Expand Down Expand Up @@ -135,6 +147,10 @@ function InternalApp(): ReactElement {
: DEFAULT_TAB_TITLE;
}, [unreadCount]);

if (isFirefox && !isFetched) {
return null;
}

if (!hostGranted) {
return isCheckingHostPermissions ? null : <ExtensionPermissionsPrompt />;
}
Expand All @@ -143,6 +159,18 @@ function InternalApp(): ReactElement {
return <ExtensionOnboarding />;
}

if (isFirefox && firefoxPermission !== FirefoxPermissionType.Accepted) {
if (firefoxPermission === FirefoxPermissionType.Declined) {
return (
<FirefoxPermissionDeclined
onGoBack={() => setFirefoxPermission(null)}
/>
);
}

return <FirefoxPermission onUpdate={setFirefoxPermission} />;
}

return (
<DndContextProvider>
<MainFeedPage onPageChanged={onPageChanged} />
Expand Down
91 changes: 91 additions & 0 deletions packages/extension/src/permission/FirefoxPermission.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import React, { ReactElement } from 'react';
import DailyDevLogo from '@dailydotdev/shared/src/components/Logo';
import {
Typography,
TypographyColor,
TypographyTag,
TypographyType,
} from '@dailydotdev/shared/src/components/typography/Typography';
import {
Button,
ButtonVariant,
} from '@dailydotdev/shared/src/components/buttons/Button';
import { VIcon } from '@dailydotdev/shared/src/components/icons';
import { cloudinaryFeedBgLaptop } from '@dailydotdev/shared/src/lib/image';
import { privacyPolicy } from '@dailydotdev/shared/src/lib/constants';
import { FirefoxPermissionItem } from './FirefoxPermissionItem';
import { FirefoxPermissionContainer } from './common';

export enum FirefoxPermissionType {
Accepted = 'accepted',
Declined = 'declined',
}

interface FirefoxPermissionProps {
onUpdate(permission: FirefoxPermissionType): void;
}

export function FirefoxPermission({
onUpdate,
}: FirefoxPermissionProps): ReactElement {
return (
<FirefoxPermissionContainer>
<img
src={cloudinaryFeedBgLaptop}
alt="a glowing background"
className="pointer-events-none absolute -top-10 -z-1 select-none"
/>
<span className="py-6">
<DailyDevLogo />
</span>
<div className="flex w-full max-w-[33rem] flex-col gap-2 rounded-10 bg-surface-float p-4">
<Typography tag={TypographyTag.H1} type={TypographyType.Title3} bold>
Additional permissions needed
</Typography>
<Typography
type={TypographyType.Callout}
color={TypographyColor.Tertiary}
>
Ready to go? 🟢 Give us the green signal by accepting the below
permissions. Enhance your daily.dev experience with personalized
content and connect with fellow developers.
</Typography>
<FirefoxPermissionItem
className="mt-2"
title="Ads pixel tracking"
description="Enable pixel tracking for fewer, non-intrusive ads, helping us in keeping the experience free and relevant."
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe raise with product if they don't want to add people can also buy plus and no ads are shown here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Let me check.

icon="📢"
/>
<FirefoxPermissionItem
title="Analytics data"
description="Allow us to capture analytics data to understand your preferences and personalize content for you."
icon="📊"
/>
<Typography
tag={TypographyTag.Link}
className="my-4 text-center"
color={TypographyColor.Link}
type={TypographyType.Subhead}
href={privacyPolicy}
>
Full permission document
</Typography>
<Button
className="w-full"
icon={<VIcon />}
variant={ButtonVariant.Primary}
onClick={() => onUpdate(FirefoxPermissionType.Accepted)}
>
Accept
</Button>
<Button
className="mt-1 w-full"
variant={ButtonVariant.Float}
onClick={() => onUpdate(FirefoxPermissionType.Declined)}
>
Decline
</Button>
</div>
</FirefoxPermissionContainer>
);
}
63 changes: 63 additions & 0 deletions packages/extension/src/permission/FirefoxPermissionDeclined.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import React, { ReactElement } from 'react';
import {
cloudinaryFeedBgLaptop,
cloudinaryGenericErrorDark,
} from '@dailydotdev/shared/src/lib/image';
import {
Typography,
TypographyColor,
TypographyTag,
TypographyType,
} from '@dailydotdev/shared/src/components/typography/Typography';
import { FirefoxPermissionContainer } from './common';

interface FirefoxPermissionDeclinedProps {
onGoBack(): void;
}

export function FirefoxPermissionDeclined({
onGoBack,
}: FirefoxPermissionDeclinedProps): ReactElement {
return (
<FirefoxPermissionContainer className="justify-center">
<img
src={cloudinaryFeedBgLaptop}
alt="a glowing background"
className="pointer-events-none absolute -top-10 -z-1 select-none"
/>
<div className="flex w-full max-w-[35rem] flex-col items-center gap-4">
<span className="h-36 max-w-[16.25rem]">
<img src={cloudinaryGenericErrorDark} alt="You declined (FML)" />
Copy link
Member

Choose a reason for hiding this comment

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

(FML) 😂

</span>
<Typography
tag={TypographyTag.H1}
type={TypographyType.LargeTitle}
bold
>
You declined.
</Typography>
<Typography
color={TypographyColor.Secondary}
tag={TypographyTag.H1}
type={TypographyType.Body}
className="text-center"
>
We understand your choice, but just so you know, we’ll need those
permissions to deliver any content. Without them, it’s like trying to
code without a keyboard!
</Typography>
<Typography
color={TypographyColor.Secondary}
tag={TypographyTag.H1}
type={TypographyType.Body}
>
You can{' '}
<button type="button" className="text-text-link" onClick={onGoBack}>
go back
Copy link
Member

Choose a reason for hiding this comment

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

Since the user has already declined, maybe we should also show a link to the webapp as an alternative?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I can also raise it to the product team 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

Added! 🚀

</button>{' '}
Copy link
Contributor

@AmarTrebinjac AmarTrebinjac Nov 21, 2024

Choose a reason for hiding this comment

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

Are these intentional or just the linter? if you wrap "you can" and "and accept the required permissions." in spans, then just having a regular space between the elements will separate the words naturally. But it should also work to just have a regular space without any wrapping or {' '}

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, intentional by the linter :lolsob: , they get joined together without it. Not normally a fan adding a wrap without any additional attributes, but I'm open to it 👀

and accept the required permissions.
</Typography>
</div>
</FirefoxPermissionContainer>
);
}
48 changes: 48 additions & 0 deletions packages/extension/src/permission/FirefoxPermissionItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import React, { ReactElement, ReactNode } from 'react';
import classNames from 'classnames';
import {
Typography,
TypographyColor,
TypographyTag,
TypographyType,
} from '@dailydotdev/shared/src/components/typography/Typography';

interface FirefoxPermissionItemProps {
title: string;
description: string;
icon: ReactNode;
className?: string;
}

export function FirefoxPermissionItem({
title,
description,
icon,
className,
}: FirefoxPermissionItemProps): ReactElement {
return (
<div
className={classNames(
'flex flex-row gap-3 border-b border-border-subtlest-tertiary px-4 py-2',
className,
)}
>
<Typography type={TypographyType.LargeTitle}>{icon}</Typography>
<div className="flex flex-1 flex-col gap-2 break-words">
<Typography
tag={TypographyTag.H3}
type={TypographyType.Callout}
color={TypographyColor.Secondary}
>
{title}
</Typography>
<Typography
type={TypographyType.Subhead}
color={TypographyColor.Tertiary}
>
{description}
</Typography>
</div>
</div>
);
}
6 changes: 6 additions & 0 deletions packages/extension/src/permission/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import classed from '@dailydotdev/shared/src/lib/classed';

export const FirefoxPermissionContainer = classed(
'div',
'fixed inset-0 z-max flex h-screen w-screen flex-1 flex-col items-center',
);
4 changes: 4 additions & 0 deletions packages/shared/src/lib/func.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ export enum UserAgent {
export const checkIsBrowser = (agent: UserAgent): boolean =>
globalThis?.navigator?.userAgent?.includes(agent);

export const checkIsFirefox = (): boolean =>
typeof globalThis?.navigator !== 'undefined' &&
/firefox/i.test(globalThis?.navigator.userAgent);

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maybe add "firefox" to the UserAgent enum and use checkIsBrowser instead?

Copy link
Member Author

@sshanzel sshanzel Nov 22, 2024

Choose a reason for hiding this comment

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

Yeah, I thought of it at first but decided to use the one specifically for Firefox. Maybe yeah it is better to keep them on the same function by lowercasing the values from the userAgent.

export const checkIsChromeOnly = (): boolean =>
checkIsBrowser(UserAgent.Chrome) && !checkIsBrowser(UserAgent.Edge);

Expand Down