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

Refactor os & mobile util functions #7303

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Refactor os & mobile util functions #7303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b9575b
refactor: clean up os and mobile util functions
Oct 9, 2024
8430fdc
fix: tests after utils refactor
Oct 9, 2024
42721a8
Merge branch 'master' into cadeban/utils2
cadeban Oct 17, 2024
b21b0e6
Merge branch 'master' into cadeban/utils2
cadeban Oct 29, 2024
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
36 changes: 16 additions & 20 deletions web/src/components/AppStoreBanner.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Capacitor } from '@capacitor/core';
import { render, renderHook, screen } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import { useAtom } from 'jotai';
Expand All @@ -6,17 +7,20 @@ import { describe, expect, test, vi } from 'vitest';

import { AppStoreBanner, appStoreDismissedAtom, AppStoreURLs } from './AppStoreBanner';

const mocks = vi.hoisted(() => ({
isMobileWeb: vi.fn(),
isAndroid: vi.fn(),
isIphone: vi.fn(),
vi.mock('@capacitor/core', () => ({
Capacitor: {
isNativePlatform: vi.fn(),
},
}));

vi.mock('features/weather-layers/wind-layer/util', () => mocks);
Object.defineProperty(navigator, 'userAgent', {
value: 'android',
configurable: true,
});

describe('AppStoreBanner', () => {
beforeEach(() => {
mocks.isMobileWeb.mockReturnValue(true);
vi.mocked(Capacitor.isNativePlatform).mockReturnValue(false);
});
afterEach(() => {
vi.restoreAllMocks();
Expand All @@ -25,8 +29,6 @@ describe('AppStoreBanner', () => {
});

test('renders when isAppBannerDismissed is undefined', () => {
mocks.isAndroid.mockReturnValue(true);

render(<AppStoreBanner />);

expect(screen.getByRole('banner')).toBeDefined();
Expand All @@ -43,8 +45,6 @@ describe('AppStoreBanner', () => {
});

test('clicking dismiss sets local storage', async () => {
mocks.isAndroid.mockReturnValue(true);

render(<AppStoreBanner />);

await userEvent.click(screen.getByTestId('dismiss-btn'));
Expand All @@ -53,8 +53,6 @@ describe('AppStoreBanner', () => {
});

test('clicking cta button sets local storage', async () => {
mocks.isAndroid.mockReturnValue(true);

render(<AppStoreBanner />);

await userEvent.click(screen.getByText('app-banner.cta'));
Expand All @@ -63,27 +61,25 @@ describe('AppStoreBanner', () => {
});

test('cta href should match on android', () => {
mocks.isAndroid.mockReturnValue(true);
mocks.isIphone.mockReturnValue(false);

render(<AppStoreBanner />);

const node = screen.getByText('app-banner.cta');
expect(node.getAttribute('href')).toBe(AppStoreURLs.GOOGLE);
});

test('cta href should match on iphone', () => {
mocks.isAndroid.mockReturnValue(false);
mocks.isIphone.mockReturnValue(true);

Object.defineProperty(navigator, 'userAgent', {
value: 'iPhone',
configurable: true,
});
render(<AppStoreBanner />);

const node = screen.getByText('app-banner.cta');
expect(node.getAttribute('href')).toBe(AppStoreURLs.APPLE);
});

test('does not render if isMobileWeb is false', () => {
mocks.isMobileWeb.mockReturnValue(false);
test('does not render if on native app', () => {
vi.mocked(Capacitor.isNativePlatform).mockReturnValue(true);

render(<AppStoreBanner />);

Expand Down
12 changes: 4 additions & 8 deletions web/src/components/AppStoreBanner.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Capacitor } from '@capacitor/core';
import { EmapsIcon } from 'icons/emapsIcon';
import { useAtom } from 'jotai';
import { atomWithStorage } from 'jotai/utils';
Expand All @@ -6,11 +7,6 @@ import { useTranslation } from 'react-i18next';
import trackEvent from 'utils/analytics';
import { TrackEvent } from 'utils/constants';

import {
isAndroid,
isIphone,
isMobileWeb,
} from '../features/weather-layers/wind-layer/util';
import { Button } from './Button';
import { DefaultCloseButton, DefaultCloseButtonProps } from './DefaultCloseButton';

Expand Down Expand Up @@ -90,15 +86,15 @@ const useAppStoreBannerState = (): AppStoreBannerState => {
};

const getAppStoreUrl = (appStoreIsDismissed: boolean): AppStoreURLs | undefined => {
if (appStoreIsDismissed || !isMobileWeb()) {
if (appStoreIsDismissed || Capacitor.isNativePlatform()) {
return undefined;
}

if (isAndroid()) {
if (/Android|android/.test(navigator.userAgent)) {
return AppStoreURLs.GOOGLE;
}

if (isIphone()) {
if (/iPad|iPhone|iPod/.test(navigator.userAgent)) {
VIKTORVAV99 marked this conversation as resolved.
Show resolved Hide resolved
return AppStoreURLs.APPLE;
}
};
15 changes: 7 additions & 8 deletions web/src/features/panels/zone/ShareButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@ Object.defineProperty(window.navigator, 'clipboard', {
});

const mocks = vi.hoisted(() => ({
isMobile: vi.fn(),
isIos: vi.fn(),
hasMobileUserAgent: vi.fn(),
}));

vi.mock('features/weather-layers/wind-layer/util', () => mocks);
vi.mock('utils/helpers', () => mocks);

describe('ShareButton', () => {
afterEach(() => {
Expand All @@ -39,7 +38,7 @@ describe('ShareButton', () => {

test('uses Capacitor Share if on mobile and can share', async () => {
vi.mocked(Share.canShare).mockResolvedValue({ value: true });
mocks.isMobile.mockReturnValue(true);
mocks.hasMobileUserAgent.mockReturnValue(true);
render(
<ToastProvider>
<ShareButton />
Expand All @@ -53,7 +52,7 @@ describe('ShareButton', () => {
test('does not display error toast on share abort', async () => {
vi.mocked(Share.canShare).mockResolvedValue({ value: true });
vi.mocked(Share.share).mockRejectedValue(new Error('AbortError'));
mocks.isMobile.mockReturnValue(true);
mocks.hasMobileUserAgent.mockReturnValue(true);
render(
<ToastProvider>
<ShareButton />
Expand All @@ -69,7 +68,7 @@ describe('ShareButton', () => {
test('does not display error toast on share cancel', async () => {
vi.mocked(Share.canShare).mockResolvedValue({ value: true });
vi.mocked(Share.share).mockRejectedValue(new Error('Share canceled'));
mocks.isMobile.mockReturnValue(true);
mocks.hasMobileUserAgent.mockReturnValue(true);
render(
<ToastProvider>
<ShareButton />
Expand All @@ -85,7 +84,7 @@ describe('ShareButton', () => {
test('displays error toast on share error', async () => {
vi.mocked(Share.canShare).mockResolvedValue({ value: true });
vi.mocked(Share.share).mockRejectedValue(new Error('Error!'));
mocks.isMobile.mockReturnValue(true);
mocks.hasMobileUserAgent.mockReturnValue(true);
render(
<ToastProvider>
<ShareButton />
Expand All @@ -100,7 +99,7 @@ describe('ShareButton', () => {
describe('copies to clipboard', () => {
test('if not on mobile', async () => {
vi.mocked(Share.canShare).mockResolvedValue({ value: true });
mocks.isMobile.mockReturnValue(false);
mocks.hasMobileUserAgent.mockReturnValue(false);
render(
<ToastProvider>
<ShareButton />
Expand Down
8 changes: 4 additions & 4 deletions web/src/features/panels/zone/ShareButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { Capacitor } from '@capacitor/core';
import { Share as CapShare } from '@capacitor/share';
import { Button, ButtonProps } from 'components/Button';
import { MemoizedShareIcon } from 'components/ShareIcon';
import { defaultShouldShowIosIcon, MemoizedShareIcon } from 'components/ShareIcon';
import { Toast, useToastReference } from 'components/Toast';
import { isIos, isMobile } from 'features/weather-layers/wind-layer/util';
import { useShare } from 'hooks/useShare';
import { Link } from 'lucide-react';
import { useCallback, useState } from 'react';
import { useTranslation } from 'react-i18next';
import { twMerge } from 'tailwind-merge';
import { ShareType, trackShare } from 'utils/analytics';
import { baseUrl, DEFAULT_ICON_SIZE, DEFAULT_TOAST_DURATION } from 'utils/constants';
import { hasMobileUserAgent as _hasMobileUserAgent } from 'utils/helpers';

interface ShareButtonProps
extends Omit<
Expand All @@ -28,8 +28,8 @@ const trackShareClick = trackShare(ShareType.SHARE);
export function ShareButton({
iconSize = DEFAULT_ICON_SIZE,
shareUrl = baseUrl,
showIosIcon = isIos(),
hasMobileUserAgent = isMobile(),
showIosIcon = defaultShouldShowIosIcon(),
hasMobileUserAgent = _hasMobileUserAgent(),
...restProps
}: ShareButtonProps) {
const { t } = useTranslation();
Expand Down
85 changes: 1 addition & 84 deletions web/src/features/weather-layers/wind-layer/util.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Capacitor } from '@capacitor/core';
import { describe, expect, it } from 'vitest';

import { floorModulus, isIphone, isMobile, isValue } from './util';
import { floorModulus, isValue } from './util';

describe('isValue', () => {
it.each([
Expand Down Expand Up @@ -38,85 +37,3 @@ describe('floorModulus', () => {
expect(floorModulus(a, n)).toEqual(expected);
});
});

describe('isMobile', () => {
it.each([
{
userAgent:
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_3_1 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.0 Mobile/14E304 Safari/602.1',
expected: true,
},
{
userAgent:
'Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Mobile Safari/537.36',
expected: true,
},
{
userAgent:
'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Safari/537.36',
expected: false,
},
{
userAgent:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.0.3 Safari/605.1.15',
expected: false,
},
])('should return $expected for userAgent $userAgent', ({ userAgent, expected }) => {
Object.defineProperty(navigator, 'userAgent', {
value: userAgent,
configurable: true,
});
expect(isMobile()).toEqual(expected);
});
});

describe('isIphone', () => {
it.each([
{
platform: 'ios',
userAgent:
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_3_1 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.0 Mobile/14E304 Safari/602.1',
expected: true,
},
{
platform: 'ios',
userAgent:
'Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Mobile Safari/537.36',
expected: true,
},
{
platform: 'android',
userAgent:
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_3_1 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.0 Mobile/14E304 Safari/602.1',
expected: true,
},
{
platform: 'android',
userAgent:
'Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Mobile Safari/537.36',
expected: false,
},
{
platform: 'web',
userAgent:
'Mozilla/5.0 (iPhone; CPU iPhone OS 10_3_1 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.0 Mobile/14E304 Safari/602.1',
expected: true,
},
{
platform: 'web',
userAgent:
'Mozilla/5.0 (Linux; Android 6.0; Nexus 5 Build/MRA58N) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.77 Mobile Safari/537.36',
expected: false,
},
])(
'should return $expected for platform $platform and userAgent $userAgent',
({ platform, userAgent, expected }) => {
vi.spyOn(Capacitor, 'getPlatform').mockReturnValue(platform);
Object.defineProperty(navigator, 'userAgent', {
value: userAgent,
configurable: true,
});
expect(isIphone()).toEqual(expected);
}
);
});
35 changes: 0 additions & 35 deletions web/src/features/weather-layers/wind-layer/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { Capacitor } from '@capacitor/core';

/**
* @returns {Boolean} true if the specified value is not null and not undefined.
*/
Expand All @@ -14,36 +12,3 @@ export function isValue(x: unknown) {
export function floorModulus(a: number, n: number) {
return a - n * Math.floor(a / n);
}

/**
* @returns {Boolean} true if agent is probably a mobile device.
*/
export function isMobile() {
return /android|blackberry|iemobile|ipad|iphone|ipod|opera mini|webos/i.test(
navigator.userAgent
);
}

Comment on lines -21 to -25
Copy link
Member

Choose a reason for hiding this comment

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

Very nice to see this gone, was quite confusing as we have a useIsMobile hook as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🥾

/**
* @returns {Boolean} true if agent is probably an iPhone.
*/
export function isIphone() {
return (
Capacitor.getPlatform() === 'ios' || /iPad|iPhone|iPod/.test(navigator.userAgent)
);
}

/**
* @returns {Boolean} true if agent is probably an Android.
*/
export function isAndroid() {
return Capacitor.getPlatform() === 'android' || /Android/.test(navigator.userAgent);
}

export function isMobileWeb() {
return Capacitor.getPlatform() === 'web' && (isIphone() || isAndroid() || isMobile());
}

export function isIos() {
return /Mac/.test(navigator.userAgent) || isIphone();
}
4 changes: 2 additions & 2 deletions web/src/features/weather-layers/wind-layer/windy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
*/

import { GfsForecastResponse } from 'api/getWeatherData';
import { hasMobileUserAgent } from 'utils/helpers';

import { buildBounds, distort, WindVector } from './calc';
import Field from './Field';
import Grid from './Grid';
import { MAX_WIND, windIntensityColorScale } from './scales';
import { isMobile } from './util';

const VELOCITY_SCALE = 1 / 50_000; //1/70000 // scale for wind velocity (completely arbitrary--this value looks nice)
const MAX_WIND_INTENSITY = MAX_WIND; // wind velocity at which particle intensity is maximum (m/s)
Expand Down Expand Up @@ -139,7 +139,7 @@ export class Windy {
let particleCount = Math.round(
bounds.width * PARTICLE_MULTIPLIER * this.zoomScaling()
);
if (isMobile()) {
if (hasMobileUserAgent()) {
particleCount *= PARTICLE_REDUCTION;
}

Expand Down
2 changes: 1 addition & 1 deletion web/src/translation/i18n.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { setTag } from '@sentry/react';
import i18n, { t } from 'i18next';
import i18n from 'i18next';
import LanguageDetector from 'i18next-browser-languagedetector';
import resourcesToBackend from 'i18next-resources-to-backend';
import { initReactI18next } from 'react-i18next';
Expand Down
Loading
Loading