Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mobile: generate profile tab icon from user sigil #3281

Merged
merged 4 commits into from
Feb 27, 2024

Conversation

alecananian
Copy link
Collaborator

PR Checklist

  • Includes changes to desk files
  • Describes how you tested the PR locally (test ship vs livenet)
  • If a new feature, includes automated tests
  • Comments added anywhere logic may be confusing without context

Changes

  • Updates profile tab icon to be an Urbit sigil for the logged in user's ship
  • Fixes Tamagui dark mode
Screenshot 2024-02-22 at 10 07 04 PM

Note: very simple implementation for now that doesn't have all the different sizing and color options we'll need. We can address the sizing when we start to use the sigil component on other tabs, and the colors when we get contacts syncing hooked up.

Copy link

linear bot commented Feb 23, 2024

@dnbrwstr
Copy link
Collaborator

This looks great! What do you think about moving the sigil wrapper to the shared ui lib? Seems like a pretty core component.

Copy link
Collaborator

@dnbrwstr dnbrwstr left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments on styling consistency.

apps/tlon-mobile/src/components/UrbitSigil.tsx Outdated Show resolved Hide resolved
apps/tlon-mobile/src/components/UrbitSigil.tsx Outdated Show resolved Hide resolved
@dnbrwstr
Copy link
Collaborator

Although re-reading the above, maybe you're just wanting to hold off on styling w Tamagui until we build out a bit more -- fine with me if that's the case.

@alecananian
Copy link
Collaborator Author

This looks great! What do you think about moving the sigil wrapper to the shared ui lib? Seems like a pretty core component.

For this, do you think we should move the memoized XML to the shared lib? The full component probably doesn't make sense because it's React Native-specific.

Although re-reading the above, maybe you're just wanting to hold off on styling w Tamagui until we build out a bit more -- fine with me if that's the case.

That was my initial thinking, but lemme dig into the Tamagui side a little today and see what I can do here 🙂

@dnbrwstr
Copy link
Collaborator

For this, do you think we should move the memoized XML to the shared lib? The full component probably doesn't make sense because it's React Native-specific.

Ultimately, the goal is for the ui package to export interfaces for core components, so that they'd be importable in other contexts (for example, another prototype/testbed app). It could export a sigil component that in the long run could be backed by .native.ts and .web.ts implementations, but for now could be native-only.

That's a secondary goal though, so feel free to ignore if it's laborious.

That was my initial thinking, but lemme dig into the Tamagui side a little today and see what I can do here 🙂

Cool :)

@dnbrwstr dnbrwstr self-requested a review February 23, 2024 17:02
@dnbrwstr
Copy link
Collaborator

Changed to approve since I don't think either of these are blockers.

@alecananian
Copy link
Collaborator Author

alecananian commented Feb 27, 2024

@dnbrwstr I've updated this to use Tamagui now
I haven't changed anything to do with the size and space - let's address that if/when we use this component anywhere else

@dnbrwstr
Copy link
Collaborator

Great, this looks good! I'm going to go ahead and merge.

@dnbrwstr dnbrwstr merged commit 1974e9b into develop Feb 27, 2024
1 check passed
@dnbrwstr dnbrwstr deleted the alec/land-1543-port-sigil-renderer-to-rn branch February 27, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants