Skip to content

fix: dynamic new tag for import button#3941

Merged
johnnyluo merged 2 commits intomainfrom
fix/import-button-lang
Mar 3, 2026
Merged

fix: dynamic new tag for import button#3941
johnnyluo merged 2 commits intomainfrom
fix/import-button-lang

Conversation

@gastonm5
Copy link
Contributor

@gastonm5 gastonm5 commented Mar 3, 2026

Description

Please include a summary of the change and which issue is fixed.

Fixes #3921

Which feature is affected?

  • Create vault ( Secure / Fast) - Please ensure you created a Secure vault & fast vault
  • Sending - Please attach a tx link here
  • Swap - Please attach a tx link for the swap here
  • New Chain / Chain related feature - Please attach tx link here

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

Screenshots (if applicable):

Additional context

Summary by CodeRabbit

Release Notes

  • New Features

    • Added dynamic "New" tag indicator on wallet import button that intelligently adapts sizing based on available space.
  • Improvements

    • Enhanced button component flexibility and layout consistency across the app.
    • Optimized iOS font rendering with improved fallback handling.

@coderabbitai
Copy link

coderabbitai bot commented Mar 3, 2026

Walkthrough

Refactors PrimaryButton and PrimaryButtonView to accept generic view builders instead of icon strings, enabling flexible composition. Updates consumers to use the new API. Adds iOS font helper and dynamic sizing logic in CreateVaultView to address language-change layout issues.

Changes

Cohort / File(s) Summary
Core Button Refactoring
VultisigApp/.../Buttons/PrimaryButton/PrimaryButton.swift, VultisigApp/.../Buttons/PrimaryButton/PrimaryButtonView.swift
Generalized PrimaryButton and PrimaryButtonView from concrete icon properties (leadingIcon/trailingIcon strings) to generic LeadingView and TrailingView types with @ViewBuilder closures. Introduced convenience initializer extensions for icon-based and view-based combinations while maintaining existing parameters and behavior.
View Consumers
VultisigApp/.../Components/ActionBanner/ActionBannerView.swift, VultisigApp/.../New Wallet/CreateVaultView.swift
Updated to use new PrimaryButton generic API. ActionBannerView conditionally renders leadingIcon via Group. CreateVaultView replaces ZStack layout with trailingView composition, adds canFitFullNewTag computed property for dynamic sizing based on localized strings and screen width, and conditionally displays "new" label.
Typography Support
VultisigApp/.../DesignSystem/Fonts/FontStyle.swift
Added platform-specific uiFont(_:) helper method for iOS that returns UIFont with system font fallback, wrapped in conditional compilation.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Changes include refactoring PrimaryButton and PrimaryButtonView to generic view-based composition, which extends beyond the specific dynamic tag fix for the import button. The refactoring of PrimaryButton/PrimaryButtonView to generic views is a larger architectural change. Consider separating this refactoring into a distinct PR or clarify if it's necessary for the dynamic tag fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: making the new tag for the import button dynamic to handle layout issues caused by language changes.
Linked Issues check ✅ Passed The PR addresses the language-related UI break by implementing dynamic sizing and conditional visibility for the new tag in the import button, directly fixing the onboarding screen layout issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/import-button-lang

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
VultisigApp/VultisigApp/DesignSystem/Fonts/FontStyle.swift (1)

21-25: Consider explicit UIKit import and weight-preserving fallback.

Two minor observations:

  1. UIFont is used without an explicit UIKit import. While this compiles because SwiftUI transitively exposes UIKit types on iOS, an explicit import would be clearer and more future-proof.

  2. The fallback .systemFont(ofSize: size) uses the default (regular) weight. If font loading ever fails for brockmanBold or brockmanSemibold, the fallback won't preserve the intended weight.

💡 Suggested improvement
 import SwiftUI
+#if os(iOS)
+import UIKit
+#endif

 enum FontStyle: String, CaseIterable {
     // ...

     `#if` os(iOS)
-    func uiFont(_ size: CGFloat) -> UIFont {
-        UIFont(name: fontName, size: size) ?? .systemFont(ofSize: size)
+    func uiFont(_ size: CGFloat) -> UIFont {
+        UIFont(name: fontName, size: size) ?? .systemFont(ofSize: size, weight: uiFontWeight)
+    }
+    
+    private var uiFontWeight: UIFont.Weight {
+        switch self {
+        case .brockmanBold: return .bold
+        case .brockmanSemibold: return .semibold
+        case .brockmanMedium, .satoshiMedium: return .medium
+        case .brockmanRegular: return .regular
+        }
     }
     `#endif`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@VultisigApp/VultisigApp/DesignSystem/Fonts/FontStyle.swift` around lines 21 -
25, Add an explicit UIKit import and update the iOS fallback in uiFont(_ size:
CGFloat) to preserve weight: import UIKit at the top, then in uiFont use
UIFont(name: fontName, size: size) ?? UIFont.systemFont(ofSize: size, weight:
<determine weight from fontName>), where you map known fontName values like
"brockmanBold" -> .bold and "brockmanSemibold" -> .semibold (default to .regular
for others); keep the function name uiFont and the fontName identifier so the
change is easy to locate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@VultisigApp/VultisigApp/Views/New` Wallet/CreateVaultView.swift:
- Around line 134-152: canFitFullNewTag currently uses
UIScreen.main.bounds.width which breaks adaptive layouts (Split View/Stage
Manager); change it to compute buttonWidth from the actual container width
passed in via layout (use GeometryReader or a PreferenceKey to capture the
parent container width) and then recompute buttonWidth = (containerWidth - 48 -
8) / 2; keep the same contentWidth calculation but reference the computed
container-derived buttonWidth in canFitFullNewTag; update any callers of
canFitFullNewTag or the view body to supply the measured container width (e.g.,
store containerWidth in state via PreferenceKey or capture it inside a
GeometryReader) so the measurement reflects the app’s usable width on iPad.

---

Nitpick comments:
In `@VultisigApp/VultisigApp/DesignSystem/Fonts/FontStyle.swift`:
- Around line 21-25: Add an explicit UIKit import and update the iOS fallback in
uiFont(_ size: CGFloat) to preserve weight: import UIKit at the top, then in
uiFont use UIFont(name: fontName, size: size) ?? UIFont.systemFont(ofSize: size,
weight: <determine weight from fontName>), where you map known fontName values
like "brockmanBold" -> .bold and "brockmanSemibold" -> .semibold (default to
.regular for others); keep the function name uiFont and the fontName identifier
so the change is easy to locate.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e9a051 and 22a0331.

📒 Files selected for processing (5)
  • VultisigApp/VultisigApp/DesignSystem/Fonts/FontStyle.swift
  • VultisigApp/VultisigApp/Views/Components/ActionBanner/ActionBannerView.swift
  • VultisigApp/VultisigApp/Views/Components/Buttons/PrimaryButton/PrimaryButton.swift
  • VultisigApp/VultisigApp/Views/Components/Buttons/PrimaryButton/PrimaryButtonView.swift
  • VultisigApp/VultisigApp/Views/New Wallet/CreateVaultView.swift

Copy link
Contributor

@johnnyluo johnnyluo left a comment

Choose a reason for hiding this comment

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

LGTM

@johnnyluo johnnyluo enabled auto-merge (squash) March 3, 2026 20:52
@johnnyluo johnnyluo merged commit b3688aa into main Mar 3, 2026
2 checks passed
@johnnyluo johnnyluo deleted the fix/import-button-lang branch March 3, 2026 21:04
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.

[BUG] - UI breaks on onboarding screen after changing language

2 participants