Skip to content
This repository was archived by the owner on Oct 11, 2026. It is now read-only.

fix(web): add top padding above page titles #195

Merged
enaboapps merged 2 commits into from
Oct 9, 2026
Merged

fix(web): add top padding above page titles #195

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
d18ee0d
fix(web): add top padding above page titles
OwenMcGirr Oct 9, 2026
d61d741
Merge branch 'main' into codex/web-top-padding-194
enaboapps Oct 9, 2026
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
15 changes: 12 additions & 3 deletions src/components/Screen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import { BlurTargetView } from 'expo-blur';
import { type PropsWithChildren, type ReactNode, useCallback, useRef, useState } from 'react';
import { AccessibilityInfo, ScrollView, View, type NativeScrollEvent, type ScrollViewProps, type NativeSyntheticEvent } from 'react-native';
import { AccessibilityInfo, Platform, ScrollView, View, type NativeScrollEvent, type ScrollViewProps, type NativeSyntheticEvent } from 'react-native';
import { SafeAreaView, useSafeAreaInsets } from 'react-native-safe-area-context';

import { AppText } from './AppText';
import { ScrollToTopButton } from './ScrollToTopButton';
import { StickyBackdrop } from './StickyBackdrop';
import { useLayout, useTheme } from '@/theme/ThemeContext';
import { spacing as spacingTokens } from '@/theme/tokens';

/**
* Native screens start below the status bar's safe-area inset. Browsers have no
* status bar, so without this the title would touch the top of the window.
*/
export function screenTopPadding(platform: string, nativeHeader: boolean): number {
return platform === 'web' && !nativeHeader ? spacingTokens.xxl : 0;
}

type ScreenProps = PropsWithChildren<{
title: string;
Expand Down Expand Up @@ -91,7 +100,7 @@ function StickyScreenContent({ children, description, headerAccessory, isExpande
ref={scrollRef}
testID="screen-scroll"
keyboardShouldPersistTaps={keyboardShouldPersistTaps}
contentContainerStyle={{ alignItems: 'center', flexGrow: 1, gap: spacing.md, paddingBottom: paddingBottom + (scrollToTop ? 48 + spacing.md : 0), paddingHorizontal }}
contentContainerStyle={{ alignItems: 'center', flexGrow: 1, gap: spacing.md, paddingBottom: paddingBottom + (scrollToTop ? 48 + spacing.md : 0), paddingHorizontal, paddingTop: screenTopPadding(Platform.OS, nativeHeader) }}
onScroll={handleScroll}
onScrollBeginDrag={scrollToTop ? cancelScrollToTop : undefined}
onTouchStart={scrollToTop ? cancelScrollToTop : undefined}
Expand Down Expand Up @@ -138,7 +147,7 @@ export function Screen({ title, description, headerAccessory, bottomAccessory, n
const paddingHorizontal = isExpanded ? spacing.xxl : spacing.xl;
return (
<SafeAreaView edges={nativeHeader ? [] : ['top']} style={{ backgroundColor: colors.background, flex: 1 }}>
{stickyAccessory === undefined ? <ScrollView testID="screen-scroll" keyboardShouldPersistTaps={keyboardShouldPersistTaps} contentContainerStyle={{ alignItems: 'center', flexGrow: 1, paddingBottom, paddingHorizontal }}>
{stickyAccessory === undefined ? <ScrollView testID="screen-scroll" keyboardShouldPersistTaps={keyboardShouldPersistTaps} contentContainerStyle={{ alignItems: 'center', flexGrow: 1, paddingBottom, paddingHorizontal, paddingTop: screenTopPadding(Platform.OS, nativeHeader) }}>
<View testID="screen-content" style={{ gap: spacing.xl, maxWidth: isExpanded ? 960 : 640, paddingTop: nativeHeader ? spacing.xl : 0, width: '100%' }}>
{!nativeHeader ? <ScreenHeader description={description} headerAccessory={headerAccessory} stackHeader={stackHeader} title={title} /> : null}
{children}
Expand Down
14 changes: 14 additions & 0 deletions src/components/screenTopPadding.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { screenTopPadding } from './Screen';
import { spacing } from '@/theme/tokens';

describe('screen top padding', () => {
it('adds space above web page titles, which have no status bar inset', () => {
expect(screenTopPadding('web', false)).toBe(spacing.xxl);
Comment on lines +5 to +6

@greptile-apps greptile-apps Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Rendered padding is untested

The new test checks screenTopPadding but never renders Screen. Removing paddingTop from either scroll layout leaves all 26 tests in the five checked suites passing, while browser rendering confirms that the affected title loses its 24px gap. This is a non-blocking coverage gap: the spacing fix works today, but either layout could regress unnoticed. Add rendered tests with Platform.OS set to 'web' for both plain and sticky layouts, plus a nativeHeader case.

Artifacts

▶ Recording of the check

  • Chromium rendered the real plain and sticky Screen components at head and held the comparison for eight seconds, showing both 24 px title gaps.

▶ Recording of the check

  • Chromium held each independent mutation for eight seconds, showing the affected title gap fall to 0 px while the unaffected layout stayed at 24 px.

Video preview with intact spacing

  • Captured the head recording’s rendered comparison and measurement overlay, showing both layouts retain their title spacing.

Video preview with plain layout padding removed

  • Captured the after recording’s first mutation, showing the plain title gap removed and the sticky gap preserved.

Intact head spacing comparison

  • Captured both real Screen layouts at head, showing the baseline used for both mutation comparisons.

Plain layout spacing regression

  • Captured the plain-only source mutation, showing its title moves to the top of its scrolling area.

Sticky layout spacing regression

  • Captured the sticky-only source mutation, showing its title moves to the top while the plain layout remains unchanged.

Executed verification commands

  • Ran this entry point to execute the mutation checks and Chromium captures, making the verification reproducible.

Independent mutation runner

  • Executed this script to remove each padding application separately, run existing tests, export rendered fixtures, and restore source.

Real Screen rendering fixture

  • Temporarily executed this Expo root fixture to render plain and sticky Screen components side by side without connection dependencies.

Chromium capture and geometry assertions

  • Executed this Playwright script to measure title gaps and record both conditions, confirming the rendered regression.

Final restoration and source collection script

  • Executed this script to check tracked restoration and collect the files read, preserving the source context.

Mutation verification execution record

  • Captured the runner’s actual output with command, working directory, and exit code, showing all three test conditions completed successfully.

Observed Chromium spacing measurements

  • Captured browser geometry assertions for head and both mutations, confirming the isolated 24-to-0 px regressions.

Candidate source and existing test context

  • Captured actual source reads and scoped test searches before mutation, preserving the code supporting this single candidate.

Unmodified Screen baseline

  • Ran git diff before baseline testing, confirming no Screen mutation was present.

Existing tests at intact head

  • Ran the five scoped suites at head, establishing the baseline of 26 passing tests.

Plain layout’s isolated deletion

  • Captured the applied source diff, confirming only the plain ScrollView paddingTop application was removed.

Existing tests with plain padding removed

  • Ran the same five suites under the plain mutation, showing all 26 tests missed the regression.

Sticky layout’s isolated deletion

  • Captured the applied source diff, confirming only the sticky ScrollView paddingTop application was removed.

Existing tests with sticky padding removed

  • Ran the same five suites under the sticky mutation, showing all 26 tests missed this regression too.

Web build with intact Screen padding

  • Exported the actual rendering fixture with head’s Screen code, producing the baseline browser build.

Web build with plain padding removed

  • Exported the fixture while the plain mutation was applied, producing an executable regression build.

Web build with sticky padding removed

  • Exported the fixture while the sticky mutation was applied, producing the second executable regression build.

Browser requests for intact head

  • Served the baseline export to Chromium and terminated the server after capture, recording real browser requests.

Browser requests for plain mutation

  • Served the plain-mutation export to Chromium and terminated the server after capture, recording the executed condition.

Browser requests for sticky mutation

  • Served the sticky-mutation export to Chromium and terminated the server after capture, recording the executed condition.

Tracked source restored after mutation checks

  • Ran git diff and source hashes after cleanup, confirming temporary tracked edits were removed.

Final checkout verification and files read

  • Captured the final head, clean tracked diff, artifact inventory, and contents of files read, preserving restoration and source evidence.

View artifacts

T-Rex Ran code and verified through T-Rex

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/components/screenTopPadding.test.ts
Line: 5-6

Comment:
**Rendered padding is untested**

The new test checks `screenTopPadding` but never renders `Screen`. Removing `paddingTop` from either scroll layout leaves all 26 tests in the five checked suites passing, while browser rendering confirms that the affected title loses its 24px gap. This is a non-blocking coverage gap: the spacing fix works today, but either layout could regress unnoticed. Add rendered tests with `Platform.OS` set to `'web'` for both plain and sticky layouts, plus a `nativeHeader` case.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

});

it('leaves native screens and stack-header screens unchanged', () => {
expect(screenTopPadding('ios', false)).toBe(0);
expect(screenTopPadding('android', false)).toBe(0);
expect(screenTopPadding('web', true)).toBe(0);
});
});
Loading