Skip to content

Comments

feat: implement create wallet button with figma replication and icons#549

Open
Praneetha15 wants to merge 2 commits intoGreenstand:mainfrom
Praneetha15:feat/create-wallet-button
Open

feat: implement create wallet button with figma replication and icons#549
Praneetha15 wants to merge 2 commits intoGreenstand:mainfrom
Praneetha15:feat/create-wallet-button

Conversation

@Praneetha15
Copy link

@Praneetha15 Praneetha15 commented Sep 9, 2025

Description

Please provide a clear and concise description of the changes made, including
the purpose and context.

Fixes: # (issue number)
or
Resolves: # (issue number)


Changes Made

  • Changes in apps folder (specify the app and briefly describe the
    changes):

    • Web
    • Native
  • Changes in packages folder (specify the package and briefly describe
    the changes):

    • Core

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing
    functionality to not work as expected)
  • 📝 Documentation update (changes)

Screenshots

Before After
Screenshot 2025-09-09 at 1 39 38 AM Screenshot 2025-09-09 at 1 36 17 AM

How Has This Been Tested?

  • Cypress integration
  • Cypress component tests
  • Jest unit tests

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Additional Comments

(Optional) Add any additional comments or notes for reviewers here.

@Praneetha15 Praneetha15 mentioned this pull request Sep 9, 2025
@pierrelstan pierrelstan self-requested a review September 9, 2025 23:39
Copy link
Collaborator

@pierrelstan pierrelstan left a comment

Choose a reason for hiding this comment

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

Suggestions & Improvements

@Praneetha15
Copy link
Author

@pierrelstan I made the requested changes could you please check it and let me know if any more changes are required.
Thanks!

@pierrelstan
Copy link
Collaborator

pierrelstan commented Sep 16, 2025

@Praneetha15 Could you explain why the tsconfig.json file is included in this PR?

@Praneetha15
Copy link
Author

@pierrelstan I reverted apps/native/tsconfig.json to exactly match upstream main so this PR no longer includes any tsconfig changes. I’ve also merged the latest changes from upstream/main into this branch to stay up to date. The PR now only contains the intended wallet UI updates.

@pierrelstan pierrelstan self-requested a review September 22, 2025 00:46
Copy link
Collaborator

@pierrelstan pierrelstan left a comment

Choose a reason for hiding this comment

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

Could you squash the last three commits into one using git rebase -i HEAD~(any number) and then rebase onto the main branch using git rebase origin/main? This ensures the code is up-to-date with the main branch and has a clean, linear history.

…\n\n- Replace custom SVGs with Expo Vector Icons\n- Add accessibilityRole and accessibilityLabel to buttons\n- Use Colors.background instead of hardcoded values\n- Keep PR focused and up to date with main
@Praneetha15 Praneetha15 force-pushed the feat/create-wallet-button branch from f7e07db to 4b820a0 Compare September 24, 2025 17:38
@Praneetha15
Copy link
Author

I noticed some upstream changes (workflow, .gitignore, bdd files) appeared in this branch after syncing with main. I can remove those and make the PR only contains my "native wallet" updates (apps/native/app/(tabs)/wallet/index.tsx and apps/native/constants/Colors.ts). Would you prefer I do that, or is the current diff acceptable?

@pierrelstan pierrelstan requested review from pierrelstan and removed request for pierrelstan October 1, 2025 12:57
@pierrelstan
Copy link
Collaborator

@Praneetha15 The PR should focus only on the Wallet screen changes. Please exclude any unrelated changes to keep it clean.

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