From d3abaf71961563363d19d7b467598a309eeb89d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Mon, 21 Sep 2026 05:09:48 +0200 Subject: [PATCH] fix(mobile): drop Latin uppercase letter-spacing from RTL section labels https://github.com/Kilo-Org/cloud/pull/6435 --- .../home/section-header.mounted.test.tsx | 43 ++++++-- .../src/components/home/section-header.tsx | 9 +- apps/mobile/src/components/ui/eyebrow.tsx | 9 +- .../src/components/ui/text.mounted.test.tsx | 80 ++++++++++++++ apps/mobile/src/components/ui/text.tsx | 21 +++- apps/web/playwright.setup-smoke.config.ts | 77 +++++++++++++- apps/web/src/lib/stripe-client.test.ts | 100 ++++++++++++++++++ apps/web/src/lib/stripe-client.ts | 39 +++++-- .../playwright-setup-smoke-base-url.test.ts | 95 +++++++++++++++++ 9 files changed, 448 insertions(+), 25 deletions(-) create mode 100644 apps/mobile/src/components/ui/text.mounted.test.tsx create mode 100644 apps/web/src/lib/stripe-client.test.ts create mode 100644 apps/web/src/tests/playwright-setup-smoke-base-url.test.ts diff --git a/apps/mobile/src/components/home/section-header.mounted.test.tsx b/apps/mobile/src/components/home/section-header.mounted.test.tsx index 63e2af6e55..be44d468b0 100644 --- a/apps/mobile/src/components/home/section-header.mounted.test.tsx +++ b/apps/mobile/src/components/home/section-header.mounted.test.tsx @@ -60,8 +60,6 @@ describe('SectionHeader mounted layout', () => { 'max-w-full', 'font-mono-medium', 'text-[10px]', - 'tracking-[1.5px]', - 'uppercase', 'text-muted-foreground', ]) ); @@ -81,14 +79,7 @@ describe('SectionHeader mounted layout', () => { expect.arrayContaining(['grow', 'max-w-full']) ); expect((text.props.className as string).split(' ')).toEqual( - expect.arrayContaining([ - alignment, - 'font-mono-medium', - 'text-[11px]', - 'tracking-[1.5px]', - 'uppercase', - 'text-primary', - ]) + expect.arrayContaining([alignment, 'font-mono-medium', 'text-[11px]', 'text-primary']) ); expect(text.props.numberOfLines).toBeUndefined(); expect(text.props.allowFontScaling).not.toBe(false); @@ -96,6 +87,38 @@ describe('SectionHeader mounted layout', () => { expect(text.children).toEqual(['See all']); }); + // Finding home-ar-loading: the Arabic section labels carried the Latin + // uppercase letter-spacing, whose glyph gaps break a cursive script's joins + // ('ال جلسا ت'). The display treatment is LTR-only. + it.each([ + { isRTL: false, tracked: true }, + { isRTL: true, tracked: false }, + ])('letterspaces the section labels only outside RTL (RTL=$isRTL)', ({ isRTL, tracked }) => { + i18nManager.isRTL = isRTL; + const root = mount( + createElement(SectionHeader, { + label: 'الجلسات الجارية الآن', + actionLabel: 'عرض الكل', + onActionPress: () => undefined, + }) + ); + const action = root.findByProps({ accessibilityRole: 'button' }); + const text = action.find(node => Object.is(node.type, 'Text')); + const label = root.find( + node => Object.is(node.type, 'Text') && node.children.includes('الجلسات الجارية الآن') + ); + + for (const node of [label, text]) { + const classes = (node.props.className as string).split(' '); + if (tracked) { + expect(classes).toEqual(expect.arrayContaining(['uppercase', 'tracking-[1.5px]'])); + } else { + expect(classes).not.toContain('uppercase'); + expect(classes.some(name => name.startsWith('tracking'))).toBe(false); + } + } + }); + it('keeps the complete accessible action name and activates the supplied destination', () => { function Destination() { const [showAll, setShowAll] = useState(false); diff --git a/apps/mobile/src/components/home/section-header.tsx b/apps/mobile/src/components/home/section-header.tsx index 3b74ee4b15..71623bf3c3 100644 --- a/apps/mobile/src/components/home/section-header.tsx +++ b/apps/mobile/src/components/home/section-header.tsx @@ -1,6 +1,6 @@ import { I18nManager, Pressable, View } from 'react-native'; -import { Text } from '@/components/ui/text'; +import { EYEBROW_LATIN_DISPLAY, Text } from '@/components/ui/text'; import { cn } from '@/lib/utils'; type SectionHeaderProps = { @@ -26,7 +26,12 @@ export function SectionHeader({ label, actionLabel, onActionPress }: Readonly diff --git a/apps/mobile/src/components/ui/eyebrow.tsx b/apps/mobile/src/components/ui/eyebrow.tsx index d4ded1d7b6..7162e330c8 100644 --- a/apps/mobile/src/components/ui/eyebrow.tsx +++ b/apps/mobile/src/components/ui/eyebrow.tsx @@ -6,9 +6,12 @@ import { cn } from '@/lib/utils'; type EyebrowProps = Omit, 'variant'>; /** - * Eyebrow label: mono, uppercase, 10px, letter-spaced. - * Defaults to muted color; pass a `className` with a `text-*` token to - * override (e.g. agent hue). + * Eyebrow label: mono, 10px. Defaults to muted color; pass a `className` with + * a `text-*` token to override (e.g. agent hue). + * + * The uppercase, letterspaced display treatment is LTR-only: `letter-spacing` + * breaks a cursive script's joins, so an RTL eyebrow renders the same copy + * without it (see `Text`'s eyebrow variant). */ export function Eyebrow({ className, ...props }: EyebrowProps) { return ; diff --git a/apps/mobile/src/components/ui/text.mounted.test.tsx b/apps/mobile/src/components/ui/text.mounted.test.tsx new file mode 100644 index 0000000000..39b117e005 --- /dev/null +++ b/apps/mobile/src/components/ui/text.mounted.test.tsx @@ -0,0 +1,80 @@ +import { createElement, type ReactElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { Eyebrow } from '@/components/ui/eyebrow'; +import { Text } from '@/components/ui/text'; + +const i18nManager = vi.hoisted(() => ({ isRTL: false })); +// `Text` reads the native direction at render time, so the mutable flag drives +// each render; the host text element is the assertion target. +vi.mock('react-native', () => ({ + I18nManager: i18nManager, + Text: 'Text', +})); +// `@rn-primitives/slot` ships untranspiled JSX and is only reached by `asChild`. +vi.mock('@rn-primitives/slot', () => ({ Text: 'Slot.Text' })); + +let renderer: TestRenderer.ReactTestRenderer | undefined = undefined; +function mount(element: ReactElement) { + act(() => { + renderer = TestRenderer.create(element); + }); + if (!renderer) { + throw new Error('Missing Text renderer'); + } + return renderer.root; +} + +function hostClasses(root: TestRenderer.ReactTestInstance): string[] { + const node = root.find(candidate => Object.is(candidate.type, 'Text')); + return String(node.props.className).split(' '); +} + +beforeEach(() => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + i18nManager.isRTL = false; +}); +afterEach(() => { + act(() => renderer?.unmount()); + renderer = undefined; +}); + +describe('Text eyebrow letterspacing', () => { + // Finding home-ar-loading: an Arabic section label carried the Latin + // uppercase letter-spacing and broke apart mid-word ('ال جلسا ت'). + it.each([false, true])('keeps the eyebrow display treatment in LTR only (RTL=%s)', isRTL => { + i18nManager.isRTL = isRTL; + const classes = hostClasses(mount(createElement(Text, { variant: 'eyebrow' }, 'Live now'))); + expect(classes).toEqual( + expect.arrayContaining(['font-mono-medium', 'text-[10px]', 'text-muted-foreground']) + ); + if (isRTL) { + expect(classes).not.toContain('uppercase'); + expect(classes.some(name => name.startsWith('tracking'))).toBe(false); + } else { + expect(classes).toEqual(expect.arrayContaining(['uppercase', 'tracking-[1.5px]'])); + } + }); + + it('leaves a non-eyebrow variant untouched in either direction', () => { + i18nManager.isRTL = true; + const classes = hostClasses(mount(createElement(Text, null, 'Live now'))); + expect(classes).not.toContain('uppercase'); + expect(classes.some(name => name.startsWith('tracking'))).toBe(false); + }); + + it.each([false, true])('applies the same rule to the Eyebrow component (RTL=%s)', isRTL => { + i18nManager.isRTL = isRTL; + const classes = hostClasses( + mount(createElement(Eyebrow, null, isRTL ? 'الجلسات الجارية الآن' : 'LIVE NOW')) + ); + expect(classes).toEqual(expect.arrayContaining(['font-mono-medium', 'text-[10px]'])); + if (isRTL) { + expect(classes).not.toContain('uppercase'); + expect(classes.some(name => name.startsWith('tracking'))).toBe(false); + } else { + expect(classes).toEqual(expect.arrayContaining(['uppercase', 'tracking-[1.5px]'])); + } + }); +}); diff --git a/apps/mobile/src/components/ui/text.tsx b/apps/mobile/src/components/ui/text.tsx index 1c8c749cb2..e6131fce9a 100644 --- a/apps/mobile/src/components/ui/text.tsx +++ b/apps/mobile/src/components/ui/text.tsx @@ -22,7 +22,7 @@ const textVariants = cva('text-foreground text-base font-medium', { small: 'text-sm font-medium leading-none', muted: 'text-muted-foreground text-sm', mono: 'font-mono-medium text-sm', - eyebrow: 'font-mono-medium text-[10px] uppercase tracking-[1.5px] text-muted-foreground', + eyebrow: 'font-mono-medium text-[10px] text-muted-foreground', }, }, defaultVariants: { @@ -48,6 +48,18 @@ const ARIA_LEVEL = { h4: '4', } satisfies Partial>; +/** + * The eyebrow's Latin display treatment: full capitals, letterspaced. It is an + * LTR-only addition to the variant because `letter-spacing` pulls a cursive + * script apart — an Arabic eyebrow renders 'الجلسات' as 'ال جلسا ت'. An RTL + * interface keeps the mono family, size and color and drops both classes. + * + * Exported so the eyebrow-scale labels rendered outside the variant — the + * `SectionHeader` action link — carry the identical treatment instead of a + * second copy of the class string that can drift. + */ +export const EYEBROW_LATIN_DISPLAY = 'uppercase tracking-[1.5px]'; + const TextClassContext = React.createContext(undefined); function Text({ @@ -64,7 +76,12 @@ function Text({ const Component = asChild ? Slot.Text : RNText; return ( +): string | undefined { + if (env.APP_URL_OVERRIDE) return env.APP_URL_OVERRIDE; + for (const file of APP_URL_OVERRIDE_FILES) { + const path = resolve(dir, file); + if (!existsSync(path)) continue; + let value: string | undefined; + try { + value = parse(readFileSync(path, 'utf-8')).APP_URL_OVERRIDE; + } catch { + continue; + } + if (value) return value; + } + return undefined; +} + +/** + * The base URL that drives the dev stack the smoke test runs against. + * + * The dev stack serves the origin its own env names: `resolveAppUrl` builds + * every absolute auth redirect from APP_URL_OVERRIDE (`src/lib/constants`), + * and a worktree's .env.local points that at a LAN IP for phone testing or at + * 127.0.0.1 for the device harness. A NextAuth session cookie is host-scoped, + * so browsing localhost while the server redirects to that other origin drops + * the session mid-sign-in and profile.spec.ts spends its whole timeout in + * page.waitForURL — the same localhost/127.0.0.1 trap playwright.config.ts + * documents. Follow the origin the server canonicalises to; an explicit + * PLAYWRIGHT_BASE_URL still wins. + */ +export function resolveBaseUrl( + dir: string, + env: Record = process.env +): string { + const port = env.PORT ? Number(env.PORT) : 3000; + return ( + env.PLAYWRIGHT_BASE_URL ?? + appUrlOrigin(appUrlOverrideFor(dir, env)) ?? + `http://localhost:${port}` + ); +} + +const baseURL = resolveBaseUrl(__dirname); export default defineConfig({ testDir: './tests/setup-smoke', diff --git a/apps/web/src/lib/stripe-client.test.ts b/apps/web/src/lib/stripe-client.test.ts new file mode 100644 index 0000000000..baa9cd164d --- /dev/null +++ b/apps/web/src/lib/stripe-client.test.ts @@ -0,0 +1,100 @@ +jest.mock('stripe', () => { + const list = jest.fn(); + const del = jest.fn(); + return { + __esModule: true, + default: jest.fn(() => ({ paymentMethods: { list }, customers: { del } })), + __mock: { list, del }, + }; +}); + +jest.mock('@sentry/nextjs', () => ({ captureMessage: jest.fn() })); + +let client: Awaited>; +let mockList: jest.Mock; +let mockDel: jest.Mock; +let mockCaptureMessage: jest.Mock; + +// `skipStripeApi` is read once when the module loads, and the local test env +// turns it on (apps/web/.env.test.local). Reload the module with the real +// calls enabled, so the Stripe answers below are the ones under test. +async function loadStripeClient() { + process.env.SKIP_STRIPE_API = 'false'; + jest.resetModules(); + const stripeModule = await import('@/lib/stripe-client'); + ({ list: mockList, del: mockDel } = ( + jest.requireMock('stripe') as { __mock: { list: jest.Mock; del: jest.Mock } } + ).__mock); + mockCaptureMessage = (jest.requireMock('@sentry/nextjs') as { captureMessage: jest.Mock }) + .captureMessage; + return stripeModule; +} + +beforeEach(async () => { + client = await loadStripeClient(); +}); + +/** What Stripe throws for an id its account does not contain. */ +function noSuchCustomer(customerId: string): Error { + return Object.assign(new Error(`No such customer: '${customerId}'`), { + type: 'StripeInvalidRequestError', + code: 'resource_missing', + statusCode: 404, + }); +} + +describe('hasPaymentMethodInStripe', () => { + it('reports a payment method on file', async () => { + mockList.mockResolvedValue({ data: [{ id: 'pm_1' }] }); + + await expect(client.hasPaymentMethodInStripe({ stripeCustomerId: 'cus_live' })).resolves.toBe( + true + ); + expect(mockList).toHaveBeenCalledWith({ customer: 'cus_live', type: 'card' }); + }); + + it('reports no payment method when Stripe returns none', async () => { + mockList.mockResolvedValue({ data: [] }); + + await expect(client.hasPaymentMethodInStripe({ stripeCustomerId: 'cus_live' })).resolves.toBe( + false + ); + }); + + // Regression: the profile page reads this on every request, so a stored id + // Stripe no longer knows (deleted customer, or a database pointing at a + // different Stripe account) threw out of the server render and replaced the + // whole profile with "Something went wrong". + it('treats a customer Stripe does not know as no payment method', async () => { + mockList.mockRejectedValue(noSuchCustomer('cus_setup_smoke_abc')); + + await expect( + client.hasPaymentMethodInStripe({ stripeCustomerId: 'cus_setup_smoke_abc' }) + ).resolves.toBe(false); + expect(mockCaptureMessage).toHaveBeenCalled(); + }); + + it('still surfaces every other Stripe failure', async () => { + mockList.mockRejectedValue(new Error('Invalid API Key provided')); + + await expect(client.hasPaymentMethodInStripe({ stripeCustomerId: 'cus_live' })).rejects.toThrow( + 'Invalid API Key provided' + ); + }); +}); + +describe('safeDeleteStripeCustomer', () => { + it('continues when the customer is already gone', async () => { + mockDel.mockRejectedValue(noSuchCustomer('cus_gone')); + + await expect(client.safeDeleteStripeCustomer('cus_gone')).resolves.toBeUndefined(); + }); + + it('still surfaces every other Stripe failure', async () => { + mockDel.mockRejectedValue(new Error('Invalid API Key provided')); + + await expect(client.safeDeleteStripeCustomer('cus_gone')).rejects.toThrow( + 'Invalid API Key provided' + ); + }); +}); diff --git a/apps/web/src/lib/stripe-client.ts b/apps/web/src/lib/stripe-client.ts index fb6b122fcb..b17a336564 100644 --- a/apps/web/src/lib/stripe-client.ts +++ b/apps/web/src/lib/stripe-client.ts @@ -50,11 +50,23 @@ export async function deleteStripeCustomer(stripeCustomerId: string) { await client.customers.del(stripeCustomerId); } +/** + * Stripe answers `No such customer` (code `resource_missing`) when the id we + * stored does not exist in the account the secret key belongs to — the customer + * was deleted in Stripe, or the row points at another Stripe account (a local + * database against a different key). Callers treat that as an absent customer + * rather than a failure: a missing customer owns no payment methods and needs + * no removal. + */ +function isMissingStripeCustomerError(error: unknown): boolean { + return error instanceof Error && error.message.includes('No such customer'); +} + export async function safeDeleteStripeCustomer(stripeCustomerId: string) { try { await deleteStripeCustomer(stripeCustomerId); } catch (error) { - if (error instanceof Error && error.message.includes('No such customer')) { + if (isMissingStripeCustomerError(error)) { const message = `Stripe customer ${stripeCustomerId} not found, continuing with GDPR removal`; console.log(message); captureMessage(message, { @@ -75,9 +87,24 @@ export async function hasPaymentMethodInStripe({ if (skipStripeApi) return false; // This function may become redundant if our in-db administration is accurate. - const paymentMethods = await client.paymentMethods.list({ - customer: stripeCustomerId, - type: 'card', - }); - return paymentMethods.data.length > 0; + try { + const paymentMethods = await client.paymentMethods.list({ + customer: stripeCustomerId, + type: 'card', + }); + return paymentMethods.data.length > 0; + } catch (error) { + if (!isMissingStripeCustomerError(error)) throw error; + // The profile page reads this on every render. A stored id Stripe no + // longer knows must degrade to "no payment method on file" — rethrowing + // turned the whole profile into the error boundary's "Something went + // wrong" page for that user. + const message = `Stripe customer ${stripeCustomerId} not found, treating as no payment method`; + console.log(message); + captureMessage(message, { + level: 'info', + tags: { source: 'stripe-payment-method-lookup' }, + }); + return false; + } } diff --git a/apps/web/src/tests/playwright-setup-smoke-base-url.test.ts b/apps/web/src/tests/playwright-setup-smoke-base-url.test.ts new file mode 100644 index 0000000000..97398a5ce7 --- /dev/null +++ b/apps/web/src/tests/playwright-setup-smoke-base-url.test.ts @@ -0,0 +1,95 @@ +/** + * `playwright.setup-smoke.config.ts` drives the dev stack a worktree is already + * running, so its base URL has to match the origin that stack canonicalises to. + * `scripts/dev.sh` exports `APP_URL_OVERRIDE` into the `next dev` process only, + * and Playwright loads no dotenv file, so the config has to read the same env + * files itself — otherwise a worktree whose `.env.local` points the override at + * a LAN IP or 127.0.0.1 browses localhost, drops the host-scoped session cookie + * mid-sign-in, and profile.spec.ts times out in `page.waitForURL`. + */ + +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import { resolveBaseUrl } from '../../playwright.setup-smoke.config'; + +let root: string; + +/** The throwaway worktree's `apps/web`, where both commands run from. */ +function webDir(): string { + return join(root, 'apps/web'); +} + +function writeEnvFile(relativePath: string, contents: string) { + const path = join(root, relativePath); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, contents); +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'setup-smoke-base-url-')); + mkdirSync(webDir(), { recursive: true }); +}); + +afterEach(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describe('resolveBaseUrl', () => { + it('follows an APP_URL_OVERRIDE that only a worktree env file names', () => { + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE="http://192.168.1.20:3000"\n'); + + expect(resolveBaseUrl(webDir(), {})).toBe('http://192.168.1.20:3000'); + }); + + it('reads the repo-root .env.local when apps/web has none', () => { + writeEnvFile('.env.local', 'APP_URL_OVERRIDE=http://127.0.0.1:3123\n'); + + expect(resolveBaseUrl(webDir(), {})).toBe('http://127.0.0.1:3123'); + }); + + it('reads the files in the order scripts/dev.sh reads them', () => { + writeEnvFile('apps/web/.env.development.local', 'APP_URL_OVERRIDE=http://10.0.0.5:3000\n'); + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE=http://10.0.0.9:3000\n'); + writeEnvFile('.env.local', 'APP_URL_OVERRIDE=http://10.0.0.7:3000\n'); + + expect(resolveBaseUrl(webDir(), {})).toBe('http://10.0.0.5:3000'); + + rmSync(join(webDir(), '.env.development.local')); + expect(resolveBaseUrl(webDir(), {})).toBe('http://10.0.0.9:3000'); + + rmSync(join(webDir(), '.env.local')); + expect(resolveBaseUrl(webDir(), {})).toBe('http://10.0.0.7:3000'); + }); + + it('keeps an exported APP_URL_OVERRIDE ahead of the files', () => { + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE=http://192.168.1.20:3000\n'); + + expect(resolveBaseUrl(webDir(), { APP_URL_OVERRIDE: 'http://127.0.0.1:3999' })).toBe( + 'http://127.0.0.1:3999' + ); + }); + + it('keeps an explicit PLAYWRIGHT_BASE_URL ahead of every other origin', () => { + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE=http://192.168.1.20:3000\n'); + + expect(resolveBaseUrl(webDir(), { PLAYWRIGHT_BASE_URL: 'http://localhost:3000' })).toBe( + 'http://localhost:3000' + ); + }); + + it('falls back to the dev port when no file names a usable origin', () => { + expect(resolveBaseUrl(webDir(), {})).toBe('http://localhost:3000'); + expect(resolveBaseUrl(webDir(), { PORT: '3100' })).toBe('http://localhost:3100'); + + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE=not-a-url\n'); + expect(resolveBaseUrl(webDir(), { PORT: '3100' })).toBe('http://localhost:3100'); + }); + + it('lets a later file win over a key an earlier file leaves empty', () => { + writeEnvFile('apps/web/.env.development.local', 'APP_URL_OVERRIDE=\n'); + writeEnvFile('apps/web/.env.local', 'APP_URL_OVERRIDE=http://10.0.0.9:3000\n'); + + expect(resolveBaseUrl(webDir(), {})).toBe('http://10.0.0.9:3000'); + }); +});