Skip to content

Fix: Column honours fill_height: true (iOS)#13

Closed
C-Sinclair wants to merge 1 commit into
GenericJam:masterfrom
C-Sinclair:fix/ios-column-fill-height
Closed

Fix: Column honours fill_height: true (iOS)#13
C-Sinclair wants to merge 1 commit into
GenericJam:masterfrom
C-Sinclair:fix/ios-column-fill-height

Conversation

@C-Sinclair
Copy link
Copy Markdown
Contributor

@C-Sinclair C-Sinclair commented May 18, 2026

Summary

The .column case in MobNodeView (ios/MobRootView.swift) only set
maxWidth on its frame. A Column with fill_height: true therefore
collapsed to the natural height of its children — only Box was honouring
node.fillHeight. This broke the canonical full-screen pattern:

<Column fill_width fill_height>
  <Header />
  <Scroll>...</Scroll>   # expected to flex
  <Footer />             # expected to pin to bottom
</Column>

On iOS the footer sat directly beneath the last child instead of the parent's
bottom edge.

Change

ios/MobRootView.swift, .column case:

- .frame(maxWidth: .infinity, alignment: .leading)
+ .frame(maxWidth: .infinity,
+        maxHeight: node.fillHeight ? .infinity : nil,
+        alignment: .topLeading)
  • node.fillHeight is already parsed by mob_nif.m (same prop Box uses);
    no NIF or renderer changes required.
  • Alignment switched from .leading to .topLeading so children anchor at
    the top when the column flexes. When maxHeight is nil the VStack still
    hugs its content vertically, so the default path is unchanged.

Backward compatibility

Pure addition. Default fill_height is false, matching today's intrinsic-
height behaviour. No call sites need updating.

Out of scope

  • Androidandroid/ in this repo is JNI/Zig only. Compose-side parity
    (if needed) would live in the downstream host/demo app.
  • Swift tests — there is no SwiftUI test target in this repo; per
    CLAUDE.md native UI changes are verified by mix mob.deploy --native
    plus a screenshot or Mob.Test interaction. Adding a test target is
    out of scope here.

Testing

  • Deploy a screen with <Column fill_width fill_height> containing a
    header, a flex region, and a footer inside a fixed-height parent;
    confirm the footer pins to the bottom.
  • Confirm columns without fill_height still size to intrinsic height
    (no regression on existing screens).

The .column case in MobNodeView only set maxWidth on its frame, so
a Column with fill_height: true collapsed to the natural height of
its children. This broke the canonical 'header / scroll / footer'
layout — the footer sat directly under the last child instead of
the parent's bottom edge.

Pass maxHeight: .infinity when node.fillHeight is true (mirroring
Box) and switch the frame alignment to .topLeading so children
anchor at the top when the column flexes. Default behaviour
(fill_height: false) is preserved — maxHeight stays nil and the
VStack still hugs its content vertically.
@C-Sinclair C-Sinclair marked this pull request as ready for review May 18, 2026 16:24
@GenericJam
Copy link
Copy Markdown
Owner

Thanks @C-Sinclair — landed as 27c9442 in mob 0.6.15 with your authorship preserved. Six-line Swift fix for the Column fill_height: true symmetry bug. Closing since the commit is on master.

@GenericJam GenericJam closed this May 20, 2026
GenericJam added a commit that referenced this pull request May 27, 2026
The Mix -> Igniter -> Zig migration shipped (apps build via build.zig,
mob.new generates Zig templates, release scripts run as tested Elixir).
The doc wasn't maintained past 2026-05-11, so several phases still read
"in progress" though the work landed. Add a closed/historical header
saying so, and flag the one open follow-up: the build orchestration
hardcodes the mob Swift-source list (mob_dev release path now globs via
mob_dev PR #13; mob_new's build*.zig.eex templates still list files by
name and should be globbed).

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants