From 0978f99864831ec7a8b13b04dabdf1114d3478ee Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Tue, 28 Jul 2026 20:22:22 +0100 Subject: [PATCH 1/5] Fix mobile attachment and gallery polish Signed-off-by: kenny lopez --- mobile/ios/Runner/InlinePhotoPicker.swift | 13 ++ .../ios/Runner/NativeAttachmentPopover.swift | 147 ++++++++++----- .../NativeAttachmentPopoverCoordinator.swift | 62 ++++++- mobile/ios/RunnerTests/RunnerTests.swift | 83 +++++++++ .../channel_detail_page/message_bubble.dart | 5 +- mobile/lib/features/channels/compose_bar.dart | 1 - .../channels/compose_bar/attachments.dart | 64 +++++-- .../channels/compose_bar/camera_preview.dart | 15 +- .../message_content/media_carousel.dart | 16 +- .../features/channels/thread_detail_page.dart | 5 +- .../shared/widgets/message_author_meta.dart | 2 +- .../channels/channel_detail_page_test.dart | 57 ++++++ .../features/channels/compose_bar_test.dart | 167 ++++++++++++++++++ .../channels/message_content_test.dart | 45 +++++ .../widgets/message_author_meta_test.dart | 35 ++++ 15 files changed, 652 insertions(+), 65 deletions(-) diff --git a/mobile/ios/Runner/InlinePhotoPicker.swift b/mobile/ios/Runner/InlinePhotoPicker.swift index 4b8d1365df..05a576323f 100644 --- a/mobile/ios/Runner/InlinePhotoPicker.swift +++ b/mobile/ios/Runner/InlinePhotoPicker.swift @@ -3,6 +3,14 @@ import PhotosUI import UIKit import UniformTypeIdentifiers +enum EmbeddedPhotoPickerLayout { + static func applyPreferredScale(_ zoomIn: () -> Void) { + UIView.performWithoutAnimation { + zoomIn() + } + } +} + final class InlinePhotoPickerFactory: NSObject, FlutterPlatformViewFactory { private let messenger: FlutterBinaryMessenger private weak var parentViewController: UIViewController? @@ -68,6 +76,7 @@ final class InlinePhotoPickerPlatformView: NSObject, FlutterPlatformView { } containerView.backgroundColor = .clear + containerView.clipsToBounds = true if #available(iOS 17.0, *) { installPicker() } @@ -120,6 +129,10 @@ final class InlinePhotoPickerPlatformView: NSObject, FlutterPlatformView { picker.didMove(toParent: parentViewController) } pickerViewController = picker + containerView.layoutIfNeeded() + EmbeddedPhotoPickerLayout.applyPreferredScale { + picker.zoomIn() + } } private func exportPickerResult(_ result: PHPickerResult) async throws -> String { diff --git a/mobile/ios/Runner/NativeAttachmentPopover.swift b/mobile/ios/Runner/NativeAttachmentPopover.swift index bb49ca6e46..5a20b19896 100644 --- a/mobile/ios/Runner/NativeAttachmentPopover.swift +++ b/mobile/ios/Runner/NativeAttachmentPopover.swift @@ -17,12 +17,12 @@ final class NativeAttachmentPopoverViewController: case camera } + private typealias ContentPreparation = (@escaping () -> Void) -> Void + private let channel: FlutterMethodChannel private let expandedWidth: CGFloat - private let menuSize = CGSize(width: 176, height: 208) + private let menuSize = NativeAttachmentMenuLayout.size private let expandedHeight: CGFloat = 372 - private let expandedHorizontalOffset: CGFloat = 10 - private let expandedVerticalOffset: CGFloat = 40 private let contentHost = UIView() private let cameraSession = AVCaptureSession() private let cameraOutput = AVCapturePhotoOutput() @@ -50,6 +50,7 @@ final class NativeAttachmentPopoverViewController: private var activeCameraCaptureID: Int64? private var isFinishing = false private var didNotifyDismissal = false + private var keyboardDismissalOffset: CGFloat = 0 var onDismiss: (() -> Void)? @@ -125,13 +126,26 @@ final class NativeAttachmentPopoverViewController: let stack = UIStackView() stack.axis = .vertical stack.distribution = .fillEqually + stack.spacing = NativeAttachmentMenuLayout.itemSpacing stack.translatesAutoresizingMaskIntoConstraints = false container.addSubview(stack) NSLayoutConstraint.activate([ - stack.leadingAnchor.constraint(equalTo: container.leadingAnchor), - stack.trailingAnchor.constraint(equalTo: container.trailingAnchor), - stack.topAnchor.constraint(equalTo: container.topAnchor, constant: 8), - stack.bottomAnchor.constraint(equalTo: container.bottomAnchor, constant: -8), + stack.leadingAnchor.constraint( + equalTo: container.leadingAnchor, + constant: NativeAttachmentMenuLayout.contentPadding + ), + stack.trailingAnchor.constraint( + equalTo: container.trailingAnchor, + constant: -NativeAttachmentMenuLayout.contentPadding + ), + stack.topAnchor.constraint( + equalTo: container.topAnchor, + constant: NativeAttachmentMenuLayout.contentPadding + ), + stack.bottomAnchor.constraint( + equalTo: container.bottomAnchor, + constant: -NativeAttachmentMenuLayout.contentPadding + ), ]) stack.addArrangedSubview( @@ -171,6 +185,7 @@ final class NativeAttachmentPopoverViewController: private func showPhotos() { guard surface != .photos else { return } + prepareForExpandedSurface() stopCamera() var configuration = PHPickerConfiguration(photoLibrary: .shared()) @@ -192,16 +207,14 @@ final class NativeAttachmentPopoverViewController: let container = UIView() container.backgroundColor = .clear + container.clipsToBounds = true addChild(picker) picker.view.translatesAutoresizingMaskIntoConstraints = false container.addSubview(picker.view) NSLayoutConstraint.activate([ picker.view.leadingAnchor.constraint(equalTo: container.leadingAnchor), picker.view.trailingAnchor.constraint(equalTo: container.trailingAnchor), - picker.view.topAnchor.constraint( - equalTo: container.topAnchor, - constant: -8 - ), + picker.view.topAnchor.constraint(equalTo: container.topAnchor), picker.view.bottomAnchor.constraint(equalTo: container.bottomAnchor), ]) picker.didMove(toParent: self) @@ -227,11 +240,32 @@ final class NativeAttachmentPopoverViewController: trailing: actionButton ) - transition(to: .photos, content: container) + transition( + to: .photos, + content: container, + preparation: { [weak picker] reveal in + guard let picker else { + reveal() + return + } + // PHPicker ignores scale changes while its remote grid is still + // adapting to the compact menu bounds. Give it one main-loop turn at + // the final popover size, apply the scale offscreen, then reveal it. + DispatchQueue.main.async { + picker.view.layoutIfNeeded() + EmbeddedPhotoPickerLayout.applyPreferredScale { + picker.zoomIn() + picker.view.layoutIfNeeded() + } + DispatchQueue.main.async(execute: reveal) + } + } + ) } private func showCamera() { guard surface != .camera else { return } + prepareForExpandedSurface() removePhotoPicker() let container = UIView() @@ -288,9 +322,28 @@ final class NativeAttachmentPopoverViewController: stopCamera() let menu = makeMenuView() - transition(to: .menu, content: menu) { [weak self] in - self?.removePhotoPicker() + transition( + to: .menu, + content: menu, + completion: { [weak self] in + self?.removePhotoPicker() + } + ) + } + + private func prepareForExpandedSurface() { + if + let sourceHost = popoverPresentationController?.sourceView?.superview + { + keyboardDismissalOffset = max( + keyboardDismissalOffset, + NativeAttachmentExpandedSurfaceBehavior.keyboardOverlap( + containerBounds: sourceHost.bounds, + keyboardLayoutFrame: sourceHost.keyboardLayoutGuide.layoutFrame + ) + ) } + NativeAttachmentExpandedSurfaceBehavior.dismissKeyboard(in: view.window) } private func installContent(_ content: UIView) { @@ -308,6 +361,7 @@ final class NativeAttachmentPopoverViewController: private func transition( to nextSurface: Surface, content nextView: UIView, + preparation: ContentPreparation? = nil, completion: (() -> Void)? = nil ) { let previousView = visibleContentView @@ -327,50 +381,61 @@ final class NativeAttachmentPopoverViewController: ]) contentHost.layoutIfNeeded() - let direction: CGFloat = isExpanding ? 34 : -34 - nextView.alpha = UIAccessibility.isReduceMotionEnabled ? 0 : 0.01 - nextView.transform = - UIAccessibility.isReduceMotionEnabled - ? .identity - : CGAffineTransform(translationX: direction, y: 0).scaledBy( - x: 0.97, - y: 0.97 - ) + let shouldAnimate = !UIAccessibility.isReduceMotionEnabled + // Do not expose embedded surfaces while the popover changes size. In + // particular, PHPicker visibly reflows its grid from the compact menu + // width to the expanded width if it is allowed to paint during this step. + nextView.alpha = 0 visibleContentView = nextView surface = nextSurface - let duration = UIAccessibility.isReduceMotionEnabled ? 0.16 : 0.36 + let duration = shouldAnimate ? (isExpanding ? 0.24 : 0.2) : 0 UIView.animate( withDuration: duration, delay: 0, - usingSpringWithDamping: 0.86, - initialSpringVelocity: 0.18, - options: [.beginFromCurrentState, .allowUserInteraction] + options: [ + .beginFromCurrentState, + .allowUserInteraction, + .curveEaseInOut, + ] ) { self.preferredContentSize = targetSize if let popover = self.popoverPresentationController, let sourceView = popover.sourceView { - popover.sourceRect = sourceView.bounds.offsetBy( - dx: isExpanding ? self.expandedHorizontalOffset : 0, - dy: isExpanding ? self.expandedVerticalOffset : 0 + popover.sourceRect = NativeAttachmentPopoverAnchorLayout.sourceRect( + anchorBounds: sourceView.bounds, + keyboardDismissalOffset: self.keyboardDismissalOffset, + isExpanded: isExpanding ) } previousView?.alpha = 0 - previousView?.transform = - UIAccessibility.isReduceMotionEnabled - ? .identity - : CGAffineTransform(translationX: -direction, y: 0).scaledBy( - x: 0.97, - y: 0.97 - ) - nextView.alpha = 1 - nextView.transform = .identity self.view.layoutIfNeeded() } completion: { _ in previousView?.removeFromSuperview() - previousView?.transform = .identity - completion?() + self.view.layoutIfNeeded() + + let reveal = { + UIView.animate( + withDuration: shouldAnimate ? 0.14 : 0, + delay: 0, + options: [ + .beginFromCurrentState, + .allowUserInteraction, + .curveEaseOut, + ] + ) { + nextView.alpha = 1 + } completion: { _ in + completion?() + } + } + + if let preparation { + preparation(reveal) + } else { + reveal() + } } } diff --git a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift index 4b9c32ffcd..9ec80e07f9 100644 --- a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift +++ b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift @@ -1,6 +1,43 @@ import Flutter import UIKit +enum NativeAttachmentExpandedSurfaceBehavior { + @MainActor + static func dismissKeyboard(in window: UIWindow?) { + window?.endEditing(true) + } + + static func keyboardOverlap( + containerBounds: CGRect, + keyboardLayoutFrame: CGRect + ) -> CGFloat { + guard + !keyboardLayoutFrame.isNull, + !keyboardLayoutFrame.isInfinite, + keyboardLayoutFrame.minY < containerBounds.maxY + else { + return 0 + } + return max(0, containerBounds.maxY - keyboardLayoutFrame.minY) + } +} + +enum NativeAttachmentPopoverAnchorLayout { + static let expandedVerticalOffset: CGFloat = 40 + + static func sourceRect( + anchorBounds: CGRect, + keyboardDismissalOffset: CGFloat, + isExpanded: Bool + ) -> CGRect { + anchorBounds.offsetBy( + dx: 0, + dy: keyboardDismissalOffset + + (isExpanded ? expandedVerticalOffset : 0) + ) + } +} + final class NativeAttachmentPopoverCoordinator: NSObject { private let channel: FlutterMethodChannel private weak var parentViewController: UIViewController? @@ -175,6 +212,20 @@ final class NativeAttachmentPopoverCoordinator: NSObject { } } +enum NativeAttachmentMenuLayout { + static let itemCount: CGFloat = 4 + static let contentPadding: CGFloat = 16 + static let itemHeight: CGFloat = 52 + static let itemSpacing: CGFloat = 8 + static let labelTextStyle: UIFont.TextStyle = .title3 + static let size = CGSize( + width: 216, + height: (contentPadding * 2) + + (itemHeight * itemCount) + + (itemSpacing * (itemCount - 1)) + ) +} + func makeNativeAttachmentMenuButton( title: String, symbol: String, @@ -200,7 +251,9 @@ func makeNativeAttachmentMenuButton( let titleLabel = UILabel() titleLabel.text = title titleLabel.textColor = .label - titleLabel.font = .preferredFont(forTextStyle: .body) + titleLabel.font = .preferredFont( + forTextStyle: NativeAttachmentMenuLayout.labelTextStyle + ) titleLabel.adjustsFontForContentSizeCategory = true titleLabel.textAlignment = .left titleLabel.translatesAutoresizingMaskIntoConstraints = false @@ -208,7 +261,10 @@ func makeNativeAttachmentMenuButton( button.addSubview(iconView) button.addSubview(titleLabel) NSLayoutConstraint.activate([ - iconView.leadingAnchor.constraint(equalTo: button.leadingAnchor, constant: 14), + iconView.leadingAnchor.constraint( + equalTo: button.leadingAnchor, + constant: 8 + ), iconView.centerYAnchor.constraint(equalTo: button.centerYAnchor), iconView.widthAnchor.constraint(equalToConstant: 26), titleLabel.leadingAnchor.constraint( @@ -217,7 +273,7 @@ func makeNativeAttachmentMenuButton( ), titleLabel.trailingAnchor.constraint( equalTo: button.trailingAnchor, - constant: -14 + constant: -8 ), titleLabel.centerYAnchor.constraint(equalTo: button.centerYAnchor), ]) diff --git a/mobile/ios/RunnerTests/RunnerTests.swift b/mobile/ios/RunnerTests/RunnerTests.swift index a174c78684..0d1c84a15f 100644 --- a/mobile/ios/RunnerTests/RunnerTests.swift +++ b/mobile/ios/RunnerTests/RunnerTests.swift @@ -6,6 +6,80 @@ import XCTest class RunnerTests: XCTestCase { + @MainActor + func testExpandedAttachmentSurfaceDismissesKeyboard() { + let window = KeyboardDismissalSpyWindow() + + NativeAttachmentExpandedSurfaceBehavior.dismissKeyboard(in: window) + + XCTAssertTrue(window.didForceEndEditing) + } + + func testExpandedAttachmentSurfaceMeasuresKeyboardOverlap() { + XCTAssertEqual( + NativeAttachmentExpandedSurfaceBehavior.keyboardOverlap( + containerBounds: CGRect(x: 0, y: 0, width: 390, height: 844), + keyboardLayoutFrame: CGRect( + x: 0, + y: 544, + width: 390, + height: 300 + ) + ), + 300 + ) + XCTAssertEqual( + NativeAttachmentExpandedSurfaceBehavior.keyboardOverlap( + containerBounds: CGRect(x: 0, y: 0, width: 390, height: 844), + keyboardLayoutFrame: CGRect(x: 0, y: 844, width: 390, height: 0) + ), + 0 + ) + } + + func testAttachmentMenuReturnsToKeyboardDismissedAnchor() { + let anchorBounds = CGRect(x: 0, y: 0, width: 44, height: 44) + + XCTAssertEqual( + NativeAttachmentPopoverAnchorLayout.sourceRect( + anchorBounds: anchorBounds, + keyboardDismissalOffset: 300, + isExpanded: true + ), + anchorBounds.offsetBy(dx: 0, dy: 340) + ) + XCTAssertEqual( + NativeAttachmentPopoverAnchorLayout.sourceRect( + anchorBounds: anchorBounds, + keyboardDismissalOffset: 300, + isExpanded: false + ), + anchorBounds.offsetBy(dx: 0, dy: 300) + ) + } + + func testEmbeddedPhotoPickerAppliesOneZoomInStepWithoutAnimation() { + var zoomInCalls = 0 + var animationsWereEnabled = true + + EmbeddedPhotoPickerLayout.applyPreferredScale { + zoomInCalls += 1 + animationsWereEnabled = UIView.areAnimationsEnabled + } + + XCTAssertEqual(zoomInCalls, 1) + XCTAssertFalse(animationsWereEnabled) + } + + func testNativeAttachmentMenuUsesRoomyRowsAndInsets() { + XCTAssertEqual(NativeAttachmentMenuLayout.size.width, 216) + XCTAssertEqual(NativeAttachmentMenuLayout.size.height, 264) + XCTAssertEqual(NativeAttachmentMenuLayout.contentPadding, 16) + XCTAssertEqual(NativeAttachmentMenuLayout.itemHeight, 52) + XCTAssertEqual(NativeAttachmentMenuLayout.itemSpacing, 8) + XCTAssertEqual(NativeAttachmentMenuLayout.labelTextStyle, .title3) + } + func testDynamicIslandQrScannerRecognizesTallSafeAreas() { for safeAreaTopInset in [51, 59, 62] { XCTAssertTrue( @@ -228,6 +302,15 @@ class RunnerTests: XCTestCase { } } +private final class KeyboardDismissalSpyWindow: UIWindow { + private(set) var didForceEndEditing = false + + override func endEditing(_ force: Bool) -> Bool { + didForceEndEditing = force + return true + } +} + private enum RelayImagePolicyError: Error { case invalidPng case invalidJpeg diff --git a/mobile/lib/features/channels/channel_detail_page/message_bubble.dart b/mobile/lib/features/channels/channel_detail_page/message_bubble.dart index 8e7ac95644..4a49c9b210 100644 --- a/mobile/lib/features/channels/channel_detail_page/message_bubble.dart +++ b/mobile/lib/features/channels/channel_detail_page/message_bubble.dart @@ -55,7 +55,10 @@ class _MessageBubble extends ConsumerWidget { return Material( color: Colors.transparent, borderRadius: BorderRadius.circular(Radii.md), - clipBehavior: Clip.antiAlias, + // The media carousel intentionally continues through the list's trailing + // gutter. InkWell still clips its ink to [borderRadius], while leaving + // overflowing message content visible. + clipBehavior: Clip.none, child: InkWell( key: ValueKey('message-row-${message.id}'), borderRadius: BorderRadius.circular(Radii.md), diff --git a/mobile/lib/features/channels/compose_bar.dart b/mobile/lib/features/channels/compose_bar.dart index 90a19d8d3d..fbb54842d4 100644 --- a/mobile/lib/features/channels/compose_bar.dart +++ b/mobile/lib/features/channels/compose_bar.dart @@ -735,7 +735,6 @@ class ComposeBar extends HookConsumerWidget { return; } - focusNode.unfocus(); unawaited( iosAttachmentPopover .present( diff --git a/mobile/lib/features/channels/compose_bar/attachments.dart b/mobile/lib/features/channels/compose_bar/attachments.dart index cfb541fc85..432ee68f64 100644 --- a/mobile/lib/features/channels/compose_bar/attachments.dart +++ b/mobile/lib/features/channels/compose_bar/attachments.dart @@ -2,8 +2,13 @@ part of '../compose_bar.dart'; enum _AttachmentSurface { closed, menu, camera, photos } -const _attachmentMenuWidth = 176.0; -const _attachmentMenuHeight = 208.0; +const _attachmentMenuWidth = 216.0; +const _attachmentMenuHeight = 264.0; +const _attachmentMenuPadding = Grid.xs; +const _attachmentMenuItemHeight = 52.0; +const _attachmentMenuItemSpacing = Grid.xxs; +const _attachmentMenuIconSize = 24.0; +const _attachmentMenuIconSlotWidth = 28.0; const _attachmentExpandedHeight = 372.0; class _AttachmentSurfacePanel extends HookWidget { @@ -81,10 +86,24 @@ class _AttachmentSurfacePanel extends HookWidget { final visibleExpandedSurface = renderedExpandedSurface.value ?? (isExpanded ? surface : null); + final cameraInitializationReady = + reducedMotion || + (surface == _AttachmentSurface.camera && rawProgress >= 1); final expandedContent = switch (visibleExpandedSurface) { _AttachmentSurface.camera => KeyedSubtree( key: const ValueKey('camera-preview'), - child: _InlineCameraPreview(onClose: onBack, onCapture: onCapture), + child: KeyedSubtree( + key: ValueKey( + cameraInitializationReady + ? 'camera-initialization-ready' + : 'camera-initialization-deferred', + ), + child: _InlineCameraPreview( + initializeCamera: cameraInitializationReady, + onClose: onBack, + onCapture: onCapture, + ), + ), ), _AttachmentSurface.photos => KeyedSubtree( key: const ValueKey('photo-gallery'), @@ -304,10 +323,11 @@ class _AttachmentMenu extends StatelessWidget { @override Widget build(BuildContext context) { return SizedBox( + key: const ValueKey('attachment-menu'), width: _attachmentMenuWidth, height: _attachmentMenuHeight, child: Padding( - padding: const EdgeInsets.symmetric(vertical: Grid.xxs), + padding: const EdgeInsets.all(_attachmentMenuPadding), child: Column( mainAxisSize: MainAxisSize.min, children: [ @@ -316,16 +336,19 @@ class _AttachmentMenu extends StatelessWidget { label: 'Camera', onTap: onCamera, ), + const SizedBox(height: _attachmentMenuItemSpacing), _AttachmentMenuItem( icon: LucideIcons.images, label: 'Photos', onTap: onPhotos, ), + const SizedBox(height: _attachmentMenuItemSpacing), _AttachmentMenuItem( icon: LucideIcons.video, label: 'Video', onTap: onVideo, ), + const SizedBox(height: _attachmentMenuItemSpacing), _AttachmentMenuItem( icon: LucideIcons.file, label: 'Files', @@ -352,21 +375,40 @@ class _AttachmentMenuItem extends StatelessWidget { @override Widget build(BuildContext context) { return SizedBox( - height: 48, + key: ValueKey('attachment-menu-item-${label.toLowerCase()}'), + height: _attachmentMenuItemHeight, child: Tooltip( message: label, child: InkWell( onTap: onTap, child: Padding( - padding: const EdgeInsets.symmetric(horizontal: Grid.twelve), + padding: const EdgeInsets.symmetric(horizontal: Grid.xxs), child: Row( children: [ - Icon(icon, size: 20, color: context.colors.onSurfaceVariant), + SizedBox( + key: ValueKey('attachment-menu-icon-${label.toLowerCase()}'), + width: _attachmentMenuIconSlotWidth, + child: Center( + child: Icon( + icon, + size: _attachmentMenuIconSize, + color: context.colors.onSurfaceVariant, + ), + ), + ), const SizedBox(width: Grid.xxs), - Text( - label, - style: context.textTheme.bodyLarge?.copyWith( - color: context.colors.onSurface, + Expanded( + child: Text( + label, + key: ValueKey( + 'attachment-menu-label-${label.toLowerCase()}', + ), + maxLines: 1, + overflow: TextOverflow.ellipsis, + style: context.textTheme.titleMedium?.copyWith( + color: context.colors.onSurface, + fontWeight: FontWeight.w400, + ), ), ), ], diff --git a/mobile/lib/features/channels/compose_bar/camera_preview.dart b/mobile/lib/features/channels/compose_bar/camera_preview.dart index b4a0d220a4..df0788ad57 100644 --- a/mobile/lib/features/channels/compose_bar/camera_preview.dart +++ b/mobile/lib/features/channels/compose_bar/camera_preview.dart @@ -1,10 +1,15 @@ part of '../compose_bar.dart'; class _InlineCameraPreview extends HookConsumerWidget { + final bool initializeCamera; final Future Function(XFile image) onCapture; final VoidCallback onClose; - const _InlineCameraPreview({required this.onCapture, required this.onClose}); + const _InlineCameraPreview({ + required this.initializeCamera, + required this.onCapture, + required this.onClose, + }); @override Widget build(BuildContext context, WidgetRef ref) { @@ -15,6 +20,8 @@ class _InlineCameraPreview extends HookConsumerWidget { final error = useState(null); useEffect(() { + if (!initializeCamera) return null; + var disposed = false; var generation = 0; @@ -76,7 +83,9 @@ class _InlineCameraPreview extends HookConsumerWidget { onInactive: () => unawaited(disposeCurrent()), onResume: () => unawaited(initialize()), ); - unawaited(initialize()); + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!disposed) unawaited(initialize()); + }); return () { disposed = true; @@ -86,7 +95,7 @@ class _InlineCameraPreview extends HookConsumerWidget { controllerRef.value = null; unawaited(current?.dispose() ?? Future.value()); }; - }, const []); + }, [initializeCamera]); Future capture() async { final activeController = controller.value; diff --git a/mobile/lib/features/channels/message_content/media_carousel.dart b/mobile/lib/features/channels/message_content/media_carousel.dart index 88bfed5b1d..5d173017cd 100644 --- a/mobile/lib/features/channels/message_content/media_carousel.dart +++ b/mobile/lib/features/channels/message_content/media_carousel.dart @@ -1,6 +1,7 @@ part of '../message_content.dart'; const _messageMediaCarouselHeight = 220.0; +const _messageMediaCarouselTopPadding = 2.0; @immutable class _MessageGalleryItem { @@ -141,7 +142,7 @@ class _MessageImageCarousel extends HookConsumerWidget { fontWeight: FontWeight.w400, ), ), - const SizedBox(height: Grid.half), + const SizedBox(height: Grid.half + _messageMediaCarouselTopPadding), LayoutBuilder( builder: (context, constraints) { final contentWidth = constraints.hasBoundedWidth @@ -152,12 +153,16 @@ class _MessageImageCarousel extends HookConsumerWidget { final leadingExtent = leadingOverflow; final isLeftToRight = Directionality.of(context) == TextDirection.ltr; + final itemTrailingPaddings = [ + for (var index = 0; index < items.length; index++) + index == items.length - 1 ? Grid.gutter : Grid.half, + ]; final previewDecodeWidths = [ for (var index = 0; index < items.length; index++) math.max( 1.0, carouselWidth * controller.viewportFraction - - (index == items.length - 1 ? 0 : Grid.half), + itemTrailingPaddings[index], ), ]; final devicePixelRatio = MediaQuery.devicePixelRatioOf(context); @@ -190,6 +195,10 @@ class _MessageImageCarousel extends HookConsumerWidget { height: _messageMediaCarouselHeight, child: PageView.builder( controller: controller, + allowImplicitScrolling: true, + // Keep the first image aligned with the message body, but + // allow later pages to paint through the avatar gutter as + // the row scrolls. clipBehavior: Clip.none, padEnds: false, itemCount: items.length, @@ -197,8 +206,9 @@ class _MessageImageCarousel extends HookConsumerWidget { itemBuilder: (context, index) { final item = items[index]; return Padding( + key: ValueKey('message-media-carousel-page:${item.url}'), padding: EdgeInsetsDirectional.only( - end: index == items.length - 1 ? 0 : Grid.half, + end: itemTrailingPaddings[index], ), child: Semantics( button: true, diff --git a/mobile/lib/features/channels/thread_detail_page.dart b/mobile/lib/features/channels/thread_detail_page.dart index a6d75a9f05..0babba2394 100644 --- a/mobile/lib/features/channels/thread_detail_page.dart +++ b/mobile/lib/features/channels/thread_detail_page.dart @@ -504,7 +504,10 @@ class _ThreadMessage extends ConsumerWidget { child: Material( color: Colors.transparent, borderRadius: BorderRadius.circular(Radii.md), - clipBehavior: Clip.antiAlias, + // The media carousel intentionally continues through the list's + // trailing gutter. InkWell still clips its ink to [borderRadius], + // while leaving overflowing message content visible. + clipBehavior: Clip.none, child: InkWell( key: ValueKey('thread-message-row-${message.id}'), borderRadius: BorderRadius.circular(Radii.md), diff --git a/mobile/lib/shared/widgets/message_author_meta.dart b/mobile/lib/shared/widgets/message_author_meta.dart index bc69506cc9..c7aff7df82 100644 --- a/mobile/lib/shared/widgets/message_author_meta.dart +++ b/mobile/lib/shared/widgets/message_author_meta.dart @@ -83,7 +83,7 @@ class MessageAuthorMeta extends StatelessWidget { return Row( crossAxisAlignment: CrossAxisAlignment.center, children: [ - Expanded(child: authorName), + Flexible(child: authorName), if (showUsername) ...[ const SizedBox(width: Grid.half), ConstrainedBox( diff --git a/mobile/test/features/channels/channel_detail_page_test.dart b/mobile/test/features/channels/channel_detail_page_test.dart index 2dc01a9a0b..48ad861d3a 100644 --- a/mobile/test/features/channels/channel_detail_page_test.dart +++ b/mobile/test/features/channels/channel_detail_page_test.dart @@ -749,6 +749,63 @@ void main() { ); }); + testWidgets( + 'keeps image galleries body-aligned and flush with the trailing edge', + (tester) async { + tester.view.physicalSize = const Size(400, 800); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.resetPhysicalSize); + addTearDown(tester.view.resetDevicePixelRatio); + + const firstImage = 'https://example.com/media/first.png'; + const secondImage = 'https://example.com/media/second.png'; + await tester.pumpWidget( + _buildTestable( + messages: [ + _textMsg( + id: 'gallery', + pubkey: 'alice', + content: + 'Gallery\n' + '![First]($firstImage)\n' + '![Second]($secondImage)', + extraTags: const [ + ['imeta', 'url $firstImage', 'm image/png'], + ['imeta', 'url $secondImage', 'm image/png'], + ], + ), + ], + users: const { + 'alice': UserProfile(pubkey: 'alice', displayName: 'Alice'), + }, + ), + ); + await tester.pumpAndSettle(); + + final carousel = find.byKey(const ValueKey('message-media-carousel')); + final imageCount = find.byKey( + const ValueKey('message-media-carousel-count'), + ); + final carouselRect = tester.getRect(carousel); + final imageCountRect = tester.getRect(imageCount); + + expect(carouselRect.left, imageCountRect.left); + expect(carouselRect.right, tester.view.physicalSize.width); + expect(carouselRect.top - imageCountRect.bottom, Grid.half + 2); + + final messageMaterial = find + .ancestor( + of: find.byKey(const ValueKey('message-row-gallery')), + matching: find.byType(Material), + ) + .first; + expect( + tester.widget(messageMaterial).clipBehavior, + Clip.none, + ); + }, + ); + testWidgets('uses larger participant avatars in reply summaries', ( tester, ) async { diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index b851bf0a6e..aeeafaccec 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -544,6 +544,58 @@ void main() { } }); + testWidgets('opening the native attachment popover keeps composer focus', ( + tester, + ) async { + final previousPlatform = debugDefaultTargetPlatformOverride; + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + var presentCalls = 0; + _setMockNativeAttachmentPopoverHandler((call) async { + switch (call.method) { + case 'isSupported': + return true; + case 'present': + presentCalls += 1; + return true; + case 'dismiss': + return null; + } + return null; + }); + + try { + await tester.pumpWidget( + _buildComposeBar( + uploadService: _testUploadService(nostr.Keys.generate().nsec), + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _expandComposer(tester); + await tester.enterText(find.byType(TextField), 'Hello'); + await tester.pumpAndSettle(); + + final textField = tester.widget(find.byType(TextField)); + expect(textField.focusNode?.hasFocus, isTrue); + + await tester.tap(find.byTooltip('Add attachment').hitTestable()); + await tester.pumpAndSettle(); + + expect(presentCalls, 1); + expect(textField.focusNode?.hasFocus, isTrue); + } finally { + await _sendNativeAttachmentPopoverCall(tester, 'dismissed'); + await tester.pumpWidget(const SizedBox.shrink()); + _setMockNativeAttachmentPopoverHandler(null); + debugDefaultTargetPlatformOverride = previousPlatform; + } + }); + testWidgets('disposing a non-owner keeps native popover callbacks active', ( tester, ) async { @@ -891,6 +943,121 @@ void main() { expect(find.text('Photos'), findsOneWidget); }); + testWidgets('attachment menu uses roomy rows and surrounding padding', ( + tester, + ) async { + await tester.pumpWidget( + _buildComposeBar( + uploadService: _testUploadService(nostr.Keys.generate().nsec), + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _openAttachmentMenu(tester); + + final menu = find.byKey(const ValueKey('attachment-menu')); + final rows = [ + for (final label in ['camera', 'photos', 'video', 'files']) + find.byKey(ValueKey('attachment-menu-item-$label')), + ]; + final menuRect = tester.getRect(menu); + + expect(menuRect.size, const Size(216, 264)); + for (final row in rows) { + expect(tester.getSize(row).height, 52); + expect(tester.getRect(row).left - menuRect.left, Grid.xs); + expect(menuRect.right - tester.getRect(row).right, Grid.xs); + } + for (final label in ['Camera', 'Photos', 'Video', 'Files']) { + final text = tester.widget(find.text(label)); + expect(text.style?.fontSize, 20); + } + final icons = [ + for (final label in ['camera', 'photos', 'video', 'files']) + find.byKey(ValueKey('attachment-menu-icon-$label')), + ]; + final labels = [ + for (final label in ['camera', 'photos', 'video', 'files']) + find.byKey(ValueKey('attachment-menu-label-$label')), + ]; + for (final icon in icons) { + expect(tester.getSize(icon).width, 28); + expect( + tester + .widget( + find.descendant(of: icon, matching: find.byType(Icon)), + ) + .size, + 24, + ); + } + final labelLeft = tester.getRect(labels.first).left; + for (var index = 0; index < labels.length; index += 1) { + expect(tester.getRect(labels[index]).left, labelLeft); + expect( + tester.getRect(labels[index]).center.dy, + tester.getRect(rows[index]).center.dy, + ); + } + expect(tester.getRect(rows.first).top - menuRect.top, Grid.xs); + expect(menuRect.bottom - tester.getRect(rows.last).bottom, Grid.xs); + for (var index = 1; index < rows.length; index += 1) { + expect( + tester.getRect(rows[index]).top - + tester.getRect(rows[index - 1]).bottom, + Grid.xxs, + ); + } + }); + + testWidgets('defers camera startup until the surface morph finishes', ( + tester, + ) async { + final previousPlatform = debugDefaultTargetPlatformOverride; + debugDefaultTargetPlatformOverride = TargetPlatform.android; + try { + await tester.pumpWidget( + _buildComposeBar( + uploadService: _testUploadService(nostr.Keys.generate().nsec), + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _openAttachmentMenu(tester); + await tester.tap(find.text('Camera')); + await tester.pump(); + + expect( + find.byKey(const ValueKey('camera-initialization-deferred')), + findsOneWidget, + ); + + await tester.pump(const Duration(milliseconds: 300)); + expect( + find.byKey(const ValueKey('camera-initialization-deferred')), + findsOneWidget, + ); + + await tester.pump(const Duration(milliseconds: 20)); + expect( + find.byKey(const ValueKey('camera-initialization-ready')), + findsOneWidget, + ); + } finally { + debugDefaultTargetPlatformOverride = previousPlatform; + } + }); + testWidgets('photo picker errors keep the action visible at large text', ( tester, ) async { diff --git a/mobile/test/features/channels/message_content_test.dart b/mobile/test/features/channels/message_content_test.dart index 91e31eee22..77fab8ef09 100644 --- a/mobile/test/features/channels/message_content_test.dart +++ b/mobile/test/features/channels/message_content_test.dart @@ -539,6 +539,51 @@ Photos }, ); + testWidgets( + 'keeps adjacent carousel images active and ends with a gutter', + (tester) async { + const first = 'https://example.com/media/gutter-one.png'; + const second = 'https://example.com/media/gutter-two.png'; + await tester.pumpWidget( + _testable( + const MessageContent( + content: + ''' +![image]($first) +![image]($second) +''', + tags: [ + ['imeta', 'url $first', 'm image/png'], + ['imeta', 'url $second', 'm image/png'], + ], + ), + ), + ); + await tester.pumpAndSettle(); + + final carousel = find.byKey(const ValueKey('message-media-carousel')); + final pageViewFinder = find.descendant( + of: carousel, + matching: find.byType(PageView), + ); + final pageView = tester.widget(pageViewFinder); + + expect(pageView.allowImplicitScrolling, isTrue); + expect(pageView.clipBehavior, Clip.none); + + pageView.controller!.jumpToPage(1); + await tester.pumpAndSettle(); + + final lastCard = find.byKey( + const ValueKey('message-media-carousel-item:$second'), + ); + expect( + tester.getRect(carousel).right - tester.getRect(lastCard).right, + Grid.gutter, + ); + }, + ); + testWidgets( 'jumps to a selected gallery thumbnail when motion is disabled', (tester) async { diff --git a/mobile/test/shared/widgets/message_author_meta_test.dart b/mobile/test/shared/widgets/message_author_meta_test.dart index a97af86130..3b55946d47 100644 --- a/mobile/test/shared/widgets/message_author_meta_test.dart +++ b/mobile/test/shared/widgets/message_author_meta_test.dart @@ -4,6 +4,41 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; void main() { + testWidgets('keeps the separator and timestamp next to a short name', ( + tester, + ) async { + const displayNameKey = Key('author-display-name'); + const timestampKey = Key('author-timestamp'); + + await tester.pumpWidget( + MaterialApp( + theme: AppTheme.light(), + home: const Scaffold( + body: SizedBox( + width: 300, + child: MessageAuthorMeta( + displayName: 'Alice', + timestamp: '2m', + displayNameKey: displayNameKey, + timestampKey: timestampKey, + nameColor: Colors.black, + metadataColor: Colors.grey, + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + + final displayNameRect = tester.getRect(find.byKey(displayNameKey)); + final separatorRect = tester.getRect(find.text('ยท')); + final timestampRect = tester.getRect(find.byKey(timestampKey)); + + expect(separatorRect.left - displayNameRect.right, Grid.half); + expect(timestampRect.left - separatorRect.right, Grid.half); + expect(tester.takeException(), isNull); + }); + testWidgets('reallocates unused metadata width to the display name', ( tester, ) async { From 01536e12ba72fa149916a04e3b9863c7ce4cf059 Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Tue, 28 Jul 2026 21:04:52 +0100 Subject: [PATCH 2/5] fix mobile review findings Signed-off-by: kenny lopez --- mobile/ios/Runner/NativeAttachmentPopover.swift | 1 + .../lib/features/channels/message_content/media_carousel.dart | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mobile/ios/Runner/NativeAttachmentPopover.swift b/mobile/ios/Runner/NativeAttachmentPopover.swift index 5a20b19896..915f548743 100644 --- a/mobile/ios/Runner/NativeAttachmentPopover.swift +++ b/mobile/ios/Runner/NativeAttachmentPopover.swift @@ -982,6 +982,7 @@ final class NativeAttachmentPopoverViewController: Self.removeTemporaryFiles(temporaryPaths) return } + NativeAttachmentExpandedSurfaceBehavior.dismissKeyboard(in: view.window) isFinishing = true view.isUserInteractionEnabled = false selectionGeneration += 1 diff --git a/mobile/lib/features/channels/message_content/media_carousel.dart b/mobile/lib/features/channels/message_content/media_carousel.dart index 5d173017cd..545adba36c 100644 --- a/mobile/lib/features/channels/message_content/media_carousel.dart +++ b/mobile/lib/features/channels/message_content/media_carousel.dart @@ -1,7 +1,6 @@ part of '../message_content.dart'; const _messageMediaCarouselHeight = 220.0; -const _messageMediaCarouselTopPadding = 2.0; @immutable class _MessageGalleryItem { @@ -142,7 +141,7 @@ class _MessageImageCarousel extends HookConsumerWidget { fontWeight: FontWeight.w400, ), ), - const SizedBox(height: Grid.half + _messageMediaCarouselTopPadding), + const SizedBox(height: Grid.half + Grid.quarter), LayoutBuilder( builder: (context, constraints) { final contentWidth = constraints.hasBoundedWidth From 03b1bf521329c7016df6bab513dcf92ac9d597dd Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Wed, 29 Jul 2026 01:19:28 +0100 Subject: [PATCH 3/5] fix iOS attachment menu fallback focus Signed-off-by: kenny lopez --- mobile/lib/features/channels/compose_bar.dart | 5 +- .../features/channels/compose_bar_test.dart | 46 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/mobile/lib/features/channels/compose_bar.dart b/mobile/lib/features/channels/compose_bar.dart index 40b7d8965e..6c44285a4e 100644 --- a/mobile/lib/features/channels/compose_bar.dart +++ b/mobile/lib/features/channels/compose_bar.dart @@ -766,7 +766,10 @@ class ComposeBar extends HookConsumerWidget { }), ) .then((didPresent) { - if (!didPresent && context.mounted) toggleAttachments(); + if (!didPresent && context.mounted) { + focusNode.unfocus(); + toggleAttachments(); + } }), ); } diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index f1ff346708..7068c2abfd 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -596,6 +596,52 @@ void main() { } }); + testWidgets( + 'unsupported iOS attachment popover unfocuses before fallback menu', + (tester) async { + final previousPlatform = debugDefaultTargetPlatformOverride; + debugDefaultTargetPlatformOverride = TargetPlatform.iOS; + _setMockNativeAttachmentPopoverHandler((call) async { + return switch (call.method) { + 'isSupported' => false, + 'dismiss' => null, + _ => null, + }; + }); + + try { + await tester.pumpWidget( + _buildComposeBar( + uploadService: _testUploadService(nostr.Keys.generate().nsec), + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _expandComposer(tester); + await tester.enterText(find.byType(TextField), 'Hello'); + await tester.pumpAndSettle(); + + final textField = tester.widget(find.byType(TextField)); + expect(textField.focusNode?.hasFocus, isTrue); + + await tester.tap(find.byTooltip('Add attachment').hitTestable()); + await tester.pumpAndSettle(); + + expect(textField.focusNode?.hasFocus, isFalse); + expect(find.byKey(const ValueKey('attachment-menu')), findsOneWidget); + } finally { + await tester.pumpWidget(const SizedBox.shrink()); + _setMockNativeAttachmentPopoverHandler(null); + debugDefaultTargetPlatformOverride = previousPlatform; + } + }, + ); + testWidgets('disposing a non-owner keeps native popover callbacks active', ( tester, ) async { From 18e641bc8a7e086fa84c1a8091bdf50491ae4d4b Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Wed, 29 Jul 2026 06:21:23 +0100 Subject: [PATCH 4/5] fix compact iOS attachment popover placement Signed-off-by: kenny lopez --- .../NativeAttachmentPopoverCoordinator.swift | 51 ++++++++++++++++- mobile/ios/RunnerTests/RunnerTests.swift | 57 +++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift index 9ec80e07f9..ce44542469 100644 --- a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift +++ b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift @@ -38,6 +38,34 @@ enum NativeAttachmentPopoverAnchorLayout { } } +enum NativeAttachmentPopoverPresentationLayout { + static func keyboardDismissalOffset( + sourceRect: CGRect, + containerBounds: CGRect, + safeAreaInsets: UIEdgeInsets, + keyboardLayoutFrame: CGRect, + menuHeight: CGFloat + ) -> CGFloat { + let keyboardOverlap = + NativeAttachmentExpandedSurfaceBehavior.keyboardOverlap( + containerBounds: containerBounds, + keyboardLayoutFrame: keyboardLayoutFrame + ) + guard keyboardOverlap > 0 else { return 0 } + + let availableHeight = + sourceRect.minY - (containerBounds.minY + safeAreaInsets.top) + return availableHeight >= menuHeight ? 0 : keyboardOverlap + } + + static func sourceRect( + _ sourceRect: CGRect, + keyboardDismissalOffset: CGFloat + ) -> CGRect { + sourceRect.offsetBy(dx: 0, dy: keyboardDismissalOffset) + } +} + final class NativeAttachmentPopoverCoordinator: NSObject { private let channel: FlutterMethodChannel private weak var parentViewController: UIViewController? @@ -127,13 +155,34 @@ final class NativeAttachmentPopoverCoordinator: NSObject { } let sourceView = presenter.view - let convertedRect: CGRect + var convertedRect: CGRect if let window = sourceView?.window { convertedRect = sourceView?.convert(sourceRect, from: window) ?? sourceRect } else { convertedRect = sourceRect } + if let sourceView { + let keyboardDismissalOffset = + NativeAttachmentPopoverPresentationLayout.keyboardDismissalOffset( + sourceRect: convertedRect, + containerBounds: sourceView.bounds, + safeAreaInsets: sourceView.safeAreaInsets, + keyboardLayoutFrame: sourceView.keyboardLayoutGuide.layoutFrame, + menuHeight: NativeAttachmentMenuLayout.size.height + ) + if keyboardDismissalOffset > 0 { + NativeAttachmentExpandedSurfaceBehavior.dismissKeyboard( + in: sourceView.window + ) + convertedRect = + NativeAttachmentPopoverPresentationLayout.sourceRect( + convertedRect, + keyboardDismissalOffset: keyboardDismissalOffset + ) + } + } + let anchorView = makeSourceAnchor(frame: convertedRect) sourceView?.addSubview(anchorView) sourceAnchorView = anchorView diff --git a/mobile/ios/RunnerTests/RunnerTests.swift b/mobile/ios/RunnerTests/RunnerTests.swift index 0d1c84a15f..ba13a350ed 100644 --- a/mobile/ios/RunnerTests/RunnerTests.swift +++ b/mobile/ios/RunnerTests/RunnerTests.swift @@ -58,6 +58,63 @@ class RunnerTests: XCTestCase { ) } + func testAttachmentMenuKeepsKeyboardWhenMenuFitsAboveTrigger() { + XCTAssertEqual( + NativeAttachmentPopoverPresentationLayout.keyboardDismissalOffset( + sourceRect: CGRect(x: 320, y: 480, width: 44, height: 44), + containerBounds: CGRect(x: 0, y: 0, width: 390, height: 844), + safeAreaInsets: UIEdgeInsets(top: 59, left: 0, bottom: 34, right: 0), + keyboardLayoutFrame: CGRect( + x: 0, + y: 544, + width: 390, + height: 300 + ), + menuHeight: NativeAttachmentMenuLayout.size.height + ), + 0 + ) + } + + func testAttachmentMenuDismissesKeyboardAndRepositionsInCompactHeight() { + let sourceRect = CGRect(x: 760, y: 168, width: 44, height: 44) + let keyboardDismissalOffset = + NativeAttachmentPopoverPresentationLayout.keyboardDismissalOffset( + sourceRect: sourceRect, + containerBounds: CGRect(x: 0, y: 0, width: 844, height: 390), + safeAreaInsets: UIEdgeInsets(top: 0, left: 59, bottom: 21, right: 59), + keyboardLayoutFrame: CGRect( + x: 0, + y: 228, + width: 844, + height: 162 + ), + menuHeight: NativeAttachmentMenuLayout.size.height + ) + + XCTAssertEqual(keyboardDismissalOffset, 162) + XCTAssertEqual( + NativeAttachmentPopoverPresentationLayout.sourceRect( + sourceRect, + keyboardDismissalOffset: keyboardDismissalOffset + ), + sourceRect.offsetBy(dx: 0, dy: 162) + ) + } + + func testAttachmentMenuDoesNotMoveWithoutSoftwareKeyboard() { + XCTAssertEqual( + NativeAttachmentPopoverPresentationLayout.keyboardDismissalOffset( + sourceRect: CGRect(x: 760, y: 168, width: 44, height: 44), + containerBounds: CGRect(x: 0, y: 0, width: 844, height: 390), + safeAreaInsets: UIEdgeInsets(top: 0, left: 59, bottom: 21, right: 59), + keyboardLayoutFrame: CGRect(x: 0, y: 390, width: 844, height: 0), + menuHeight: NativeAttachmentMenuLayout.size.height + ), + 0 + ) + } + func testEmbeddedPhotoPickerAppliesOneZoomInStepWithoutAnimation() { var zoomInCalls = 0 var animationsWereEnabled = true From 1b1e2c083bda35a93bdda83431404855049777a3 Mon Sep 17 00:00:00 2001 From: kenny lopez Date: Wed, 29 Jul 2026 06:53:25 +0100 Subject: [PATCH 5/5] fix attachment menu accessibility sizing Signed-off-by: kenny lopez --- .../ios/Runner/NativeAttachmentPopover.swift | 78 +++++++++++-- .../NativeAttachmentPopoverCoordinator.swift | 52 +++++++-- mobile/ios/RunnerTests/RunnerTests.swift | 54 ++++++++- .../channels/compose_bar/attachments.dart | 105 ++++++++++++------ .../features/channels/compose_bar_test.dart | 40 +++++++ 5 files changed, 270 insertions(+), 59 deletions(-) diff --git a/mobile/ios/Runner/NativeAttachmentPopover.swift b/mobile/ios/Runner/NativeAttachmentPopover.swift index 915f548743..f2f6c01df8 100644 --- a/mobile/ios/Runner/NativeAttachmentPopover.swift +++ b/mobile/ios/Runner/NativeAttachmentPopover.swift @@ -21,8 +21,8 @@ final class NativeAttachmentPopoverViewController: private let channel: FlutterMethodChannel private let expandedWidth: CGFloat - private let menuSize = NativeAttachmentMenuLayout.size - private let expandedHeight: CGFloat = 372 + private let maximumMenuHeight: CGFloat + private let expandedHeight = NativeAttachmentMenuLayout.maximumHeight private let contentHost = UIView() private let cameraSession = AVCaptureSession() private let cameraOutput = AVCapturePhotoOutput() @@ -51,14 +51,30 @@ final class NativeAttachmentPopoverViewController: private var isFinishing = false private var didNotifyDismissal = false private var keyboardDismissalOffset: CGFloat = 0 + private var menuStackHeightConstraint: NSLayoutConstraint? var onDismiss: (() -> Void)? - init(channel: FlutterMethodChannel, expandedWidth: CGFloat) { + private var menuSize: CGSize { + NativeAttachmentMenuLayout.size( + compatibleWith: traitCollection, + maximumHeight: maximumMenuHeight + ) + } + + init( + channel: FlutterMethodChannel, + expandedWidth: CGFloat, + maximumMenuHeight: CGFloat = NativeAttachmentMenuLayout.maximumHeight + ) { self.channel = channel self.expandedWidth = expandedWidth + self.maximumMenuHeight = maximumMenuHeight super.init(nibName: nil, bundle: nil) - preferredContentSize = menuSize + preferredContentSize = NativeAttachmentMenuLayout.size( + compatibleWith: .current, + maximumHeight: maximumMenuHeight + ) } @available(*, unavailable) @@ -107,6 +123,20 @@ final class NativeAttachmentPopoverViewController: stopCamera() } + override func traitCollectionDidChange( + _ previousTraitCollection: UITraitCollection? + ) { + super.traitCollectionDidChange(previousTraitCollection) + guard + previousTraitCollection?.preferredContentSizeCategory + != traitCollection.preferredContentSizeCategory + else { + return + } + + updateMenuLayout() + } + func adaptivePresentationStyle( for controller: UIPresentationController ) -> UIModalPresentationStyle { @@ -123,29 +153,45 @@ final class NativeAttachmentPopoverViewController: let container = UIView() container.translatesAutoresizingMaskIntoConstraints = false + let scrollView = UIScrollView() + scrollView.alwaysBounceVertical = false + scrollView.translatesAutoresizingMaskIntoConstraints = false + container.addSubview(scrollView) + let stack = UIStackView() stack.axis = .vertical stack.distribution = .fillEqually stack.spacing = NativeAttachmentMenuLayout.itemSpacing stack.translatesAutoresizingMaskIntoConstraints = false - container.addSubview(stack) + scrollView.addSubview(stack) + let stackHeightConstraint = stack.heightAnchor.constraint( + equalToConstant: NativeAttachmentMenuLayout.itemsHeight( + compatibleWith: traitCollection + ) + ) + menuStackHeightConstraint = stackHeightConstraint NSLayoutConstraint.activate([ + scrollView.leadingAnchor.constraint(equalTo: container.leadingAnchor), + scrollView.trailingAnchor.constraint(equalTo: container.trailingAnchor), + scrollView.topAnchor.constraint(equalTo: container.topAnchor), + scrollView.bottomAnchor.constraint(equalTo: container.bottomAnchor), stack.leadingAnchor.constraint( - equalTo: container.leadingAnchor, + equalTo: scrollView.frameLayoutGuide.leadingAnchor, constant: NativeAttachmentMenuLayout.contentPadding ), stack.trailingAnchor.constraint( - equalTo: container.trailingAnchor, + equalTo: scrollView.frameLayoutGuide.trailingAnchor, constant: -NativeAttachmentMenuLayout.contentPadding ), stack.topAnchor.constraint( - equalTo: container.topAnchor, + equalTo: scrollView.contentLayoutGuide.topAnchor, constant: NativeAttachmentMenuLayout.contentPadding ), stack.bottomAnchor.constraint( - equalTo: container.bottomAnchor, + equalTo: scrollView.contentLayoutGuide.bottomAnchor, constant: -NativeAttachmentMenuLayout.contentPadding ), + stackHeightConstraint, ]) stack.addArrangedSubview( @@ -183,6 +229,16 @@ final class NativeAttachmentPopoverViewController: return container } + private func updateMenuLayout() { + menuStackHeightConstraint?.constant = + NativeAttachmentMenuLayout.itemsHeight( + compatibleWith: traitCollection + ) + if surface == .menu { + preferredContentSize = menuSize + } + } + private func showPhotos() { guard surface != .photos else { return } prepareForExpandedSurface() @@ -332,9 +388,7 @@ final class NativeAttachmentPopoverViewController: } private func prepareForExpandedSurface() { - if - let sourceHost = popoverPresentationController?.sourceView?.superview - { + if let sourceHost = popoverPresentationController?.sourceView?.superview { keyboardDismissalOffset = max( keyboardDismissalOffset, NativeAttachmentExpandedSurfaceBehavior.keyboardOverlap( diff --git a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift index ce44542469..73559d7c2b 100644 --- a/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift +++ b/mobile/ios/Runner/NativeAttachmentPopoverCoordinator.swift @@ -169,7 +169,9 @@ final class NativeAttachmentPopoverCoordinator: NSObject { containerBounds: sourceView.bounds, safeAreaInsets: sourceView.safeAreaInsets, keyboardLayoutFrame: sourceView.keyboardLayoutGuide.layoutFrame, - menuHeight: NativeAttachmentMenuLayout.size.height + menuHeight: NativeAttachmentMenuLayout.size( + compatibleWith: sourceView.traitCollection + ).height ) if keyboardDismissalOffset > 0 { NativeAttachmentExpandedSurfaceBehavior.dismissKeyboard( @@ -264,15 +266,51 @@ final class NativeAttachmentPopoverCoordinator: NSObject { enum NativeAttachmentMenuLayout { static let itemCount: CGFloat = 4 static let contentPadding: CGFloat = 16 - static let itemHeight: CGFloat = 52 + static let minimumItemHeight: CGFloat = 52 static let itemSpacing: CGFloat = 8 + static let itemVerticalPadding: CGFloat = 8 + static let maximumHeight: CGFloat = 372 + static let width: CGFloat = 216 static let labelTextStyle: UIFont.TextStyle = .title3 - static let size = CGSize( - width: 216, - height: (contentPadding * 2) - + (itemHeight * itemCount) + + static func itemHeight( + compatibleWith traitCollection: UITraitCollection + ) -> CGFloat { + let labelHeight = UIFont.preferredFont( + forTextStyle: labelTextStyle, + compatibleWith: traitCollection + ).lineHeight + return max( + minimumItemHeight, + ceil(labelHeight + (itemVerticalPadding * 2)) + ) + } + + static func itemsHeight( + compatibleWith traitCollection: UITraitCollection + ) -> CGFloat { + (itemHeight(compatibleWith: traitCollection) * itemCount) + (itemSpacing * (itemCount - 1)) - ) + } + + static func contentHeight( + compatibleWith traitCollection: UITraitCollection + ) -> CGFloat { + (contentPadding * 2) + itemsHeight(compatibleWith: traitCollection) + } + + static func size( + compatibleWith traitCollection: UITraitCollection, + maximumHeight: CGFloat = NativeAttachmentMenuLayout.maximumHeight + ) -> CGSize { + CGSize( + width: width, + height: min( + contentHeight(compatibleWith: traitCollection), + maximumHeight + ) + ) + } } func makeNativeAttachmentMenuButton( diff --git a/mobile/ios/RunnerTests/RunnerTests.swift b/mobile/ios/RunnerTests/RunnerTests.swift index ba13a350ed..c5333cfdf2 100644 --- a/mobile/ios/RunnerTests/RunnerTests.swift +++ b/mobile/ios/RunnerTests/RunnerTests.swift @@ -70,7 +70,11 @@ class RunnerTests: XCTestCase { width: 390, height: 300 ), - menuHeight: NativeAttachmentMenuLayout.size.height + menuHeight: NativeAttachmentMenuLayout.size( + compatibleWith: UITraitCollection( + preferredContentSizeCategory: .large + ) + ).height ), 0 ) @@ -89,7 +93,11 @@ class RunnerTests: XCTestCase { width: 844, height: 162 ), - menuHeight: NativeAttachmentMenuLayout.size.height + menuHeight: NativeAttachmentMenuLayout.size( + compatibleWith: UITraitCollection( + preferredContentSizeCategory: .large + ) + ).height ) XCTAssertEqual(keyboardDismissalOffset, 162) @@ -109,7 +117,11 @@ class RunnerTests: XCTestCase { containerBounds: CGRect(x: 0, y: 0, width: 844, height: 390), safeAreaInsets: UIEdgeInsets(top: 0, left: 59, bottom: 21, right: 59), keyboardLayoutFrame: CGRect(x: 0, y: 390, width: 844, height: 0), - menuHeight: NativeAttachmentMenuLayout.size.height + menuHeight: NativeAttachmentMenuLayout.size( + compatibleWith: UITraitCollection( + preferredContentSizeCategory: .large + ) + ).height ), 0 ) @@ -129,14 +141,44 @@ class RunnerTests: XCTestCase { } func testNativeAttachmentMenuUsesRoomyRowsAndInsets() { - XCTAssertEqual(NativeAttachmentMenuLayout.size.width, 216) - XCTAssertEqual(NativeAttachmentMenuLayout.size.height, 264) + let traits = UITraitCollection(preferredContentSizeCategory: .large) + let size = NativeAttachmentMenuLayout.size(compatibleWith: traits) + + XCTAssertEqual(size.width, 216) + XCTAssertEqual(size.height, 264) XCTAssertEqual(NativeAttachmentMenuLayout.contentPadding, 16) - XCTAssertEqual(NativeAttachmentMenuLayout.itemHeight, 52) + XCTAssertEqual( + NativeAttachmentMenuLayout.itemHeight(compatibleWith: traits), + 52 + ) XCTAssertEqual(NativeAttachmentMenuLayout.itemSpacing, 8) XCTAssertEqual(NativeAttachmentMenuLayout.labelTextStyle, .title3) } + func testNativeAttachmentMenuGrowsAndScrollsForAccessibilityText() { + let traits = UITraitCollection( + preferredContentSizeCategory: .accessibilityExtraExtraExtraLarge + ) + let itemHeight = NativeAttachmentMenuLayout.itemHeight( + compatibleWith: traits + ) + let contentHeight = NativeAttachmentMenuLayout.contentHeight( + compatibleWith: traits + ) + let size = NativeAttachmentMenuLayout.size(compatibleWith: traits) + + XCTAssertGreaterThan(itemHeight, 52) + XCTAssertGreaterThan(contentHeight, 264) + XCTAssertEqual( + size.height, + min(contentHeight, NativeAttachmentMenuLayout.maximumHeight) + ) + XCTAssertLessThanOrEqual( + size.height, + NativeAttachmentMenuLayout.maximumHeight + ) + } + func testDynamicIslandQrScannerRecognizesTallSafeAreas() { for safeAreaTopInset in [51, 59, 62] { XCTAssertTrue( diff --git a/mobile/lib/features/channels/compose_bar/attachments.dart b/mobile/lib/features/channels/compose_bar/attachments.dart index 351e9f6e9c..7c53ae1098 100644 --- a/mobile/lib/features/channels/compose_bar/attachments.dart +++ b/mobile/lib/features/channels/compose_bar/attachments.dart @@ -3,7 +3,6 @@ part of '../compose_bar.dart'; enum _AttachmentSurface { closed, menu, camera, photos } const _attachmentMenuWidth = 216.0; -const _attachmentMenuHeight = 264.0; const _attachmentMenuPadding = Grid.xs; const _attachmentMenuItemHeight = 52.0; const _attachmentMenuItemSpacing = Grid.xxs; @@ -11,6 +10,45 @@ const _attachmentMenuIconSize = 24.0; const _attachmentMenuIconSlotWidth = 28.0; const _attachmentExpandedHeight = 372.0; +@immutable +class _AttachmentMenuLayout { + final double itemHeight; + final double contentHeight; + final double height; + + const _AttachmentMenuLayout({ + required this.itemHeight, + required this.contentHeight, + required this.height, + }); + + factory _AttachmentMenuLayout.from(BuildContext context) { + final textPainter = TextPainter( + text: TextSpan(text: 'Camera', style: context.textTheme.titleMedium), + textDirection: Directionality.of(context), + textScaler: MediaQuery.textScalerOf(context), + maxLines: 1, + )..layout(); + final itemHeight = math.max( + _attachmentMenuItemHeight, + textPainter.height + (Grid.xxs * 2), + ); + textPainter.dispose(); + final contentHeight = + (_attachmentMenuPadding * 2) + + (itemHeight * 4) + + (_attachmentMenuItemSpacing * 3); + + return _AttachmentMenuLayout( + itemHeight: itemHeight, + contentHeight: contentHeight, + height: math.min(contentHeight, _attachmentExpandedHeight), + ); + } + + bool get isScrollable => contentHeight > height; +} + class _AttachmentSurfacePanel extends HookWidget { final _AttachmentSurface surface; final Widget suggestionPanel; @@ -41,6 +79,7 @@ class _AttachmentSurfacePanel extends HookWidget { Widget build(BuildContext context) { if (surface == _AttachmentSurface.closed) return suggestionPanel; + final menuLayout = _AttachmentMenuLayout.from(context); final reducedMotion = MediaQuery.disableAnimationsOf(context); final isExpanded = surface == _AttachmentSurface.camera || @@ -136,8 +175,8 @@ class _AttachmentSurfacePanel extends HookWidget { _attachmentMenuWidth + ((expandedWidth - _attachmentMenuWidth) * sizeProgress); final height = - _attachmentMenuHeight + - ((expandedHeight - _attachmentMenuHeight) * sizeProgress); + menuLayout.height + + ((expandedHeight - menuLayout.height) * sizeProgress); final baseColor = context.colors.surfaceContainerHighest; final expandedColor = visibleExpandedSurface == _AttachmentSurface.camera @@ -170,12 +209,13 @@ class _AttachmentSurfacePanel extends HookWidget { left: 0, top: 0, width: _attachmentMenuWidth, - height: _attachmentMenuHeight, + height: menuLayout.height, child: IgnorePointer( ignoring: surface != _AttachmentSurface.menu, child: Opacity( opacity: menuOpacity, child: _AttachmentMenu( + layout: menuLayout, onCamera: onCamera, onPhotos: onPhotos, onVideo: onVideo, @@ -308,12 +348,14 @@ class _AttachmentTrigger extends StatelessWidget { } class _AttachmentMenu extends StatelessWidget { + final _AttachmentMenuLayout layout; final VoidCallback onCamera; final VoidCallback onPhotos; final VoidCallback onVideo; final VoidCallback onFiles; const _AttachmentMenu({ + required this.layout, required this.onCamera, required this.onPhotos, required this.onVideo, @@ -325,48 +367,43 @@ class _AttachmentMenu extends StatelessWidget { return SizedBox( key: const ValueKey('attachment-menu'), width: _attachmentMenuWidth, - height: _attachmentMenuHeight, - child: Padding( + height: layout.height, + child: ListView.separated( + key: const ValueKey('attachment-menu-scroll'), padding: const EdgeInsets.all(_attachmentMenuPadding), - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - _AttachmentMenuItem( - icon: LucideIcons.camera, - label: 'Camera', - onTap: onCamera, - ), - const SizedBox(height: _attachmentMenuItemSpacing), - _AttachmentMenuItem( - icon: LucideIcons.images, - label: 'Photos', - onTap: onPhotos, - ), - const SizedBox(height: _attachmentMenuItemSpacing), - _AttachmentMenuItem( - icon: LucideIcons.video, - label: 'Video', - onTap: onVideo, - ), + physics: layout.isScrollable + ? null + : const NeverScrollableScrollPhysics(), + itemCount: 4, + separatorBuilder: (_, _) => const SizedBox(height: _attachmentMenuItemSpacing), - _AttachmentMenuItem( - icon: LucideIcons.file, - label: 'Files', - onTap: onFiles, - ), - ], - ), + itemBuilder: (context, index) { + final (icon, label, onTap) = switch (index) { + 0 => (LucideIcons.camera, 'Camera', onCamera), + 1 => (LucideIcons.images, 'Photos', onPhotos), + 2 => (LucideIcons.video, 'Video', onVideo), + _ => (LucideIcons.file, 'Files', onFiles), + }; + return _AttachmentMenuItem( + height: layout.itemHeight, + icon: icon, + label: label, + onTap: onTap, + ); + }, ), ); } } class _AttachmentMenuItem extends StatelessWidget { + final double height; final IconData icon; final String label; final VoidCallback onTap; const _AttachmentMenuItem({ + required this.height, required this.icon, required this.label, required this.onTap, @@ -376,7 +413,7 @@ class _AttachmentMenuItem extends StatelessWidget { Widget build(BuildContext context) { return SizedBox( key: ValueKey('attachment-menu-item-${label.toLowerCase()}'), - height: _attachmentMenuItemHeight, + height: height, child: Tooltip( message: label, child: InkWell( diff --git a/mobile/test/features/channels/compose_bar_test.dart b/mobile/test/features/channels/compose_bar_test.dart index 7068c2abfd..7b747adfac 100644 --- a/mobile/test/features/channels/compose_bar_test.dart +++ b/mobile/test/features/channels/compose_bar_test.dart @@ -1320,6 +1320,46 @@ void main() { } }); + testWidgets( + 'attachment menu grows rows and scrolls for accessibility text', + (tester) async { + await tester.pumpWidget( + _buildComposeBar( + uploadService: _testUploadService(nostr.Keys.generate().nsec), + textScaler: const TextScaler.linear(4), + onSend: + ( + content, + mentionPubkeys, { + mediaTags = const >[], + }) async {}, + ), + ); + + await _openAttachmentMenu(tester); + + final menu = find.byKey(const ValueKey('attachment-menu')); + final rows = [ + for (final label in ['camera', 'photos', 'video', 'files']) + find.byKey(ValueKey('attachment-menu-item-$label')), + ]; + final scrollView = tester.widget( + find.byKey(const ValueKey('attachment-menu-scroll')), + ); + + expect(tester.getSize(menu), const Size(216, 372)); + expect(tester.getSize(rows.first).height, greaterThan(52)); + expect(scrollView.physics, isA()); + await tester.drag( + find.byKey(const ValueKey('attachment-menu-scroll')), + const Offset(0, -300), + ); + await tester.pump(); + expect(tester.getSize(rows.last).height, greaterThan(52)); + expect(tester.takeException(), isNull); + }, + ); + testWidgets('defers camera startup until the surface morph finishes', ( tester, ) async {