Repository navigation
Host a public-API native glass surface behind the app tab bar - #216
Conversation
iebb
left a comment
There was a problem hiding this comment.
Reviewed exact head 17eb4d4 against master 066c2d6; changed-file blobs independently matched. The surface-only layout is a legitimate improvement and the 23 supplied/adjacent Flutter tests plus scoped formatting, analysis, and generated/localization checks pass. However this is not ready for iOS publication.
Actual Swift type-checking against the installed iPhoneSimulator SDK and cached Flutter framework fails independently of the Dart mocks. GlassSurfaceHost.init(frame:) requires override; Self.registerLumaOverride references no member because registerLumaOverride is a file-level constant. Command: xcrun swiftc -typecheck -swift-version 5 -sdk <iPhoneSimulator27.0.sdk> -target arm64-apple-ios26.0-simulator -F ios/Runner/NativeGlassSurface.swift. This is a compiler check, not a simulator/device visual pass.
The new process-wide swizzle also targets a private mangled UIKit class and backdropLayer:didChangeLuma:, then replaces that implementation through method_setImplementation. Defensive lookup/type checks do not make it a public API. Apple guideline 2.5.1 requires public APIs. Please retain the useful surface-only geometry with a public-API implementation, remove the private hook, and provide an actual native build plus the retained iPhone 17 Pro QA for theme changes, gestures, and geometry. Copying another client's hook is not evidence of store compliance.
Leaving this legitimate PR open for revision; no release, deployment, store action, Telegram access, or speculative native redesign was performed.
|
|
||
| init(frame: CGRect) { | ||
| super.init(frame: frame) | ||
| Self.registerLumaOverride |
There was a problem hiding this comment.
[P1] The native source does not type-check: init(frame:) is overriding UIView and needs override; Self.registerLumaOverride has no such member (the declaration is file-level). Flutter method-channel mocks cannot catch either compiler error.
| } | ||
| originalLumaImplementation?(view, selector, layer, luma) | ||
| } | ||
| method_setImplementation(method, unsafeBitCast(replacement, to: IMP.self)) |
There was a problem hiding this comment.
[P1] Remove this private UIKit implementation replacement from the store-bound path. It hooks a private mangled class/selector process-wide; lookup/encoding guards do not satisfy the public-API requirement. Keep the geometry improvement using documented public APIs and validate the actual native target.
17eb4d4 to
27a010a
Compare
|
Updated in
|
iebb
left a comment
There was a problem hiding this comment.
Re-reviewed 27a010a against master f311e34 in a credential-free snapshot. The two previous Swift compiler errors are fixed. Independent xcrun swiftc -typecheck -swift-version 5 against the installed iPhoneSimulator27.0 SDK and Flutter simulator framework now succeeds (Swift-6-isolation warnings remain under Swift 5). The private UIKit class/selector lookup and method swizzle are removed; the remaining surface uses public UIKit APIs. The required declaration is complete and exact-head Quality is green.
There is a new reproducible appearance regression in NativeGlassSurface._configuration (native_glass_surface.dart:33-35): platformBrightness==dark is ORed with the app palette. With an explicitly light app theme (AppColors.light) while the OS is dark, the actual channel update sends dark:true, so Swift forces a dark native surface beneath light Flutter controls. The extended production channel test fails expected false / actual true. The prior NativeBottomTabBar derived this value solely from the effective app palette. Please respect the selected app appearance and pin both disagreement directions plus an in-place theme update.
Nine adjacent Flutter geometry/layout/motion tests pass; that and the successful Swift type check are not an integrated native build or simulator visual pass. Native theme, gesture hit testing, and geometry QA on the retained iPhone 17 Pro remain needed before merging this native host replacement. No simulator/app/account, Telegram, release, or deployment action was performed in this review. Leaving the legitimate PR open for correction and native validation.
27a010a to
b09c8e5
Compare
|
Fixed in
The channel test pins both disagreement directions plus the in-place theme flip: OS dark + app light → Native simulator/device QA on a real iOS build host remains the outstanding item, as before. |
iebb
left a comment
There was a problem hiding this comment.
Re-reviewed b09c8e5 against master 4e34da3. The app-palette fix resolves the prior OS/app light-dark mismatch; all 11 focused channel/geometry/fallback checks pass, including the independent mismatch checks. Presentation-only/public UIKit usage remains compliant/unchanged.
There is still a safe-area regression in the actual shell: the iOS branch returns NativeGlassSurface directly, without the SafeArea/insets previously provided by NativeBottomTabBar (or the fallback's SafeArea). With a 390×844 viewport and a 34px bottom safe area, the real MainSplitRootView platform surface ends at y=844 instead of at or above y=810. The 62px tab controls therefore extend into the home-indicator region. My full-shell regression fails; the supplied geometry tests use zero bottom padding and miss it. Preserve bottom and lateral safe areas and the intended capsule spacing, and cover phone insets plus narrow/split/RTL layouts.
Native platform validation is also incomplete. I attempted a credential-free, separately identified synthetic app build targeting only the retained iPhone 17 Pro simulator. Xcode dependency/build artifacts exhausted available disk space before a successful native run; this is an environment limitation, not a claimed Swift source failure. I removed only this run's regenerable native-build artifacts, recovering about 7 GB. No installed Mithka app or account was opened or changed. A successful native build/run and gesture/theme check are still required before merge. Left open for revision.
| // bottom bar or sidebar column. The Flutter glass bar remains the | ||
| // fallback for every other target. | ||
| if (!kIsWeb && defaultTargetPlatform == TargetPlatform.iOS) { | ||
| return NativeGlassSurface( |
There was a problem hiding this comment.
[P1] Restore safe-area containment around the iOS glass host. In the actual compact shell with 34px bottom padding, this platform surface still ends at the screen bottom (844), placing controls in the home-indicator region; the old host accounted for safeBottom.
The system tab bar was a window-bottom UITabBar: its selection capsule painted against its own slot metrics, so a tablet split shell's narrow sidebar column misplaced the highlight and had to opt out of the native glass (useNativeGlass:false, PR iebb#210). Replace whole-bar hosting with a glass-surface-only platform view using only documented public UIKit APIs: - NativeGlassSurface.swift hosts UIVisualEffectView(UIGlassEffect(.regular)) on iOS 26+ (system material UIBlurEffect below), styled via public overrideUserInterfaceStyle, tintColor, isInteractive = false, and layer.cornerRadius. GlassSurfaceHost overrides init(frame:); no private UIKit classes, selectors, or Objective-C runtime hooks are used. The view draws no tabs and takes no touches. - Flutter keeps the tab row, badges, and gestures, and adds the extracted GlassSelectionIndicator (from LiquidGlassBottomBar) so selection renders identically over either glass. One path now serves the phone bottom bar and the sidebar column; the split-shell opt-out and its test are gone because no system-drawn element is positioned relative to tab slots. - NativeBottomTabBar (UITabBar, Heroicons rasterizer, context menu, badge channel) is deleted with its test; the native surface test pins the new channel contract and the geometry test pins pill-over-slot in both shells.
b09c8e5 to
4c504a6
Compare
|
Fixed in Safe-area containment restored. The iOS glass host no longer returns Regression coverage added (
Local gates at the new head (CI SDK 3.44.2): |
iebb
left a comment
There was a problem hiding this comment.
Re-reviewed 4c504a6 against latest origin/master 4e34da3. The previous safe-area and effective-theme issues are resolved. No remaining correctness findings in the reviewed scope.
Independent validation on the exact proposed merged tree 3dd5414d8dec4c936d17c993cd97e3c880cf5146, using only secrets_example.dart:
- Full unit/widget suite: 3,472 passed, 1 skipped, 6m43s.
- Full flutter analyze: no issues. dart format over lib/test/integration_test/test_driver: 1,000 files, 0 changed.
- Localization/source/catalogue checks: 8 locales x 3,830 keys, 9 generated assets current; visible-string and registration-anchor checks pass.
- Compile-time guard tests: omitted via full suite, explicit false, true with expected opt-in, and invalid value all pass. No official build or guard configuration is changed.
- Prior independent real MainSplitRootView safe-area and effective app/OS palette mismatch regressions pass, alongside the expanded compact/narrow/split/RTL geometry tests (10 combined cases).
- Native QA on the retained iPhone 17 Pro simulator E6AE5DE2-B925-47DF-AFD3-BD2A5D9C0B39, iOS 27.0 / Xcode SDK 27.1: built and ran the actual unmodified Swift glass implementation and its unmodified production Dart surface/indicator dependencies in a minimal credential-free Flutter host with a separate org.iebb.review.mithka bundle. Eight configurations (light/dark, LTR/RTL, 360/280 widths), all four tab taps, long presses, indicator geometry, and disposal passed. The harness uses the same production platform-view registration. This is component-native QA, not a claimed full Mithka app build or pre-iOS-26 device run; the iOS-15 fallback compiles under the availability guard.
- Actual Xcode project/file references, AppDelegate factory/channel registration, fallback path, and removed native-tab code were inspected; no new Material/Cupertino icons/assets.
- Current-head Quality run 39020991586 is successful.
Agentic reviewer confirmation: independently confirmed compliant/unchanged Terms classification (navigation presentation only, no Telegram/TDLib behavior changed), completed basic checks, and inspected the relevant guards/evidence. Applicable checks and current-head Quality pass; no unresolved classification or unguarded section 1.4 behavior. Safe to merge this scoped change.
Change
The iOS glass bottom bar hosted a whole
UITabBarplatform view. Because the system bar is a window-bottom bar, its selection capsule painted against its own slot metrics: inside a tablet split shell's narrow sidebar column the highlight landed over the wrong place, and #210 had to add auseNativeGlassopt-out so split shells drew the app-owned bar instead — two code paths, a 202-line guard test, and phones still getting the system bar's fixed slot geometry instead of the app's label-aware layout.This PR replaces whole-bar hosting with a glass-surface-only platform view using only documented public UIKit APIs:
ios/Runner/NativeGlassSurface.swift):GlassSurfaceHost(override init(frame: CGRect)) hosts a non-interactiveUIVisualEffectViewwith the public iOS 26UIGlassEffect(style: .regular)(glass.tintColor = tint,glass.isInteractive = false) and a fallbackUIBlurEffect(style: dark ? .systemMaterialDark : .systemMaterialLight)on earlier iOS versions, styled withoverrideUserInterfaceStyle = dark ? .dark : .lightandeffectView.layer.cornerRadius = cornerRadius. No private UIKit classes, selectors, or Objective-C runtime hooks (NSClassFromString/method_setImplementation) are used. The view draws no tabs, no icons, no badges, and takes no touches (isUserInteractionEnabled = false).GlassSelectionIndicator(extracted fromLiquidGlassBottomBar) so the selection pill (borderRadius: AppRadius.control) is the same widget over either glass. One path serves the phone bottom bar and the sidebar column;useNativeGlass: false, the Heroicons rasterizer, the badge/context-menu channel, and the window-slot failure mode are all deleted (NativeBottomTabBarremoved outright).Affected platforms: iOS (UIKit host view), other targets unchanged (
LiquidGlassBottomBarfallback path and its tests untouched).Telegram API Terms classification
Follow the contribution and agentic review rules. Select exactly one classification and explain it below.
This is an agentic source-code review classification. Every store version must remain compliant with applicable Telegram API Terms; the guarded-source exception does not apply to store distribution or review.
allowTelegramTosViolations(I_DONT_FUCKING_CARE_ABOUT_TOS), disabled in official builds, and available only in an explicitly enabled self-build.Reason and affected Telegram behavior: presentation-only change to the app's own navigation chrome. No Telegram API interaction, no TDLib effects, no read/typing/online behavior touched. No §1.4 feature involved; the
allowTelegramTosViolationsguard is not applicable.Basic checks (every PR)
Check each item after verification. If a check is inapplicable, explain why in the evidence below before checking it.
dart format --output=none --set-exit-if-changedon all 5 changed.dartfiles (CI SDK 3.44.2,Formatted 5 files (0 changed)).flutter analyzeon the 5 changed Dart files (No issues found!).flutter test --concurrency=4.Commands, results, platform/build/UI evidence, and reasons for inapplicable checks:
~/.cache/dart3122/bin/dart format --output=none --set-exit-if-changed lib/app/liquid_glass_bottom_bar.dart lib/app/main_tab_view.dart lib/app/native_glass_surface.dart test/native_glass_bar_geometry_test.dart test/native_glass_surface_test.dart→Formatted 5 files (0 changed)PATH=/tmp/flutter-3.44.2/bin:$PATH flutter analyze --no-pub lib/app/liquid_glass_bottom_bar.dart lib/app/main_tab_view.dart lib/app/native_glass_surface.dart test/native_glass_bar_geometry_test.dart test/native_glass_surface_test.dart→No issues found!flutter test --no-pub -j 1 test/native_glass_surface_test.dart— 1/1 (channel contract: create params,updateon theme flip, no item data crosses, taps stay on the app row)flutter test --no-pub -j 1 test/native_glass_bar_geometry_test.dart— 2/2 (pill covers the selected slot at 900×800 split shell and 390×844 phone; tab tap still selects)flutter test --no-pub -j 1 test/liquid_glass_bottom_bar_test.dart test/bottom_bar_layout_test.dart test/radius_scale_test.dart— 7/7ios/Runner.xcodeproj/project.pbxprojupdated (fileRef/buildFile/sources-phase IDs rewritten forNativeGlassSurface.swift, zero stale references),AppDelegate.swiftregistersNativeGlassSurfaceFactoryundermithka/native_glass.python3 tool/check_l10n_strings.py→No visible UI string literals outside lib/l10n.python3 translations/tools/check.py→8 locales x 3826 keys (11 counted): OKpython3 translations/tools/gen_assets.py --check→9 locale assets are up to datepython3 tool/gen_registration_anchors.py --check→ exit 0x86_64), so macOSxcrun swiftcand iPhone 17 Pro simulator QA cannot be executed locally from this environment;NativeGlassSurface.swiftnow usesoverride init(frame: CGRect)and exclusively publicFlutterandUIKitAPIs (UIVisualEffectView,UIGlassEffect,UIBlurEffect,overrideUserInterfaceStyle).Section 1.4 exception checks
Not applicable
Agentic reviewer confirmation
Agentic code reviewer only: record this confirmation in the PR review or check these boxes before approval.