784 replace native base #806
Conversation
# Conflicts: # src/components/screens/Feed/components/CompactFeedItem/CompactFeedItemThumbnail.tsx
# Conflicts: # src/components/common/CommunityCounts.tsx # src/components/common/Markdown/RenderMarkdown.tsx # src/components/screens/Feed/CommunityAboutScreen.tsx # src/components/screens/Feed/components/Community/ModeratorList.tsx # src/components/screens/Feed/components/FeedView.tsx
Start.tsx
Outdated
| isSystemFont, | ||
| accentColor, | ||
| ]); | ||
| }, [currentTheme, fontSize, getFontScale, isSystemFont, accentColor]); |
There was a problem hiding this comment.
Should the fontSize stay in the dependencies as long as it is deactivated?
There was a problem hiding this comment.
Probably not. With font scaling commented out, it won't make a difference since the value shouldn't change, so it's just a matter of cleanup. It sounded like @gkasdorf was going to try uncomment the font scaling though.
There was a problem hiding this comment.
I'm gonna try to work on font scaling at some point soon tonight or tomorrow, can we at least leave a todo there so i remember to re-add it?
| <Spacer /> | ||
| <ChevronRightIcon /> | ||
| <Icon as={ChevronRightIcon} /> |
There was a problem hiding this comment.
Would an SFIcon work here too ?
There was a problem hiding this comment.
Yep, it should. It just passes any provided properties right through. I got it working with IonIcons.
There was a problem hiding this comment.
Yea please no icons other than SFIcons
There was a problem hiding this comment.
I was trying to do a 1:1 with Native Base. I'll see what SFIcon we have as a replacement.
There was a problem hiding this comment.
Oh I see, I thought I would've removed this but it's not a tabler icon so I didn't find it.
There was a problem hiding this comment.
There are two cases of IonIcons being used too. I didn't touch those either.
| import { Path } from "react-native-svg"; | ||
| import { Root } from "../styled-components"; | ||
|
|
||
| const FavouriteIcon: any = createIcon({ |
There was a problem hiding this comment.
That's one of the Gluestack generated component so maybe the dev was a Brit? Most of those icons can probably be deleted since I don't think we're likely to use them.
gluestack-ui.config.ts
Outdated
| colors: { | ||
| primary50: "#f0f9ff", | ||
| primary100: "#e0f2fe", | ||
| primary200: "#bae6fd", | ||
| primary300: "#7dd3fc", | ||
| primary400: "#38bdf8", | ||
| primary500: "#0ea5e9", | ||
| primary600: "#0284c7", | ||
| primary700: "#0369a1", | ||
| primary800: "#075985", | ||
| primary900: "#0c4a6e", |
There was a problem hiding this comment.
I assume these are autogenerated but do we need them all?
There was a problem hiding this comment.
Not all of them. I'll try to clean out the ones we don't use.
There was a problem hiding this comment.
We should change to completely remove using these variables but you don't have to do that here.
There was a problem hiding this comment.
Yeah, we used some random colors from Native Base so I wanted that change to be deliberate.
tsconfig.json
Outdated
| "baseUrl": "./", | ||
| "paths": { | ||
| "@root/*": [ | ||
| "./*" | ||
| ], | ||
| "@src/*": [ | ||
| "./src/*" | ||
| ], | ||
| "@components/*": [ | ||
| "./src/components/*" | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
Hmm think we could do this better unless not having webpack is causing us to do this. Ik with webpack we can do it in a cleaner way.
baseUrl: "./src",
plus probably some other config changes
There was a problem hiding this comment.
I forgot to clean that up to have a single root. Unfortunately we need to have it in both babel and tsconfig unless you know of a way for webpack to get typescript to resolve properly.
There was a problem hiding this comment.
Let me look into it a little more
There was a problem hiding this comment.
This more-or-less stayed the same. I was able to simplify the babel.config.js though so it uses what's in here to populate over there.
|
@sgriff96 @felix-staud I think I've addressed all of the feedback except for the tsconfig item. |
PR Creator Checklist
Ensure you've checked the following before submitting your PR:
Summary
Replacing Native Base with Gluestack #784 .
Everything Native Base is pulled out. Theming is working, but font scaling is not so I've commented that code out for now.
I did my best to test everything. There are likely some small things I overlooked so let me know if you see something wonky.
Screenshots
No screen shots applicable. Ideally nothing looks or acts differently than it did before.
Test Plan
Everything. All. The. Things.