PHC-4075 - Add base style [OAuthLoginButton] #64
Conversation
Pull Request Test Coverage Report for Build 4324981218
💛 - Coveralls |
namick
force-pushed
the
PHC-4075-add-basic-wireframe-styling
branch
from
1a62b3a
to
5d38319
Compare
| import React from 'react'; | ||
| import { storiesOf } from '@storybook/react-native'; | ||
| import { authConfig } from './OAuth.stories'; | ||
| import { RootProviders, RootStack } from 'src'; | ||
|
|
||
| storiesOf('Example App', module).add('DemoApp', () => ( | ||
| <RootProviders authConfig={authConfig}> | ||
| <RootStack /> | ||
| </RootProviders> | ||
| )); |
There was a problem hiding this comment.
Add the whole demo app as a storybook story.
There was a problem hiding this comment.
Nice! I had run into some SafeArea issues before where I thought the one storybook was providing & the <SafeAreaProvider> in RootProviders were fighting. Looking back, I'm betting I was instead just hitting npm / metro / path issues between example & root, and confusing the two. This will be super helpful
| /** | ||
| * Base (wireFrame-like) color scheme | ||
| * | ||
| * The base color scheme uses a popular design system to | ||
| * dynamically generate all color tokens from the HTML color: | ||
| * "gray" - #808080 - rgb(128,128,128) | ||
| */ | ||
| const scheme = { | ||
| light: { | ||
| primary: 'rgb(0, 104, 116)', | ||
| onPrimary: 'rgb(255, 255, 255)', | ||
| primaryContainer: 'rgb(151, 240, 255)', | ||
| onPrimaryContainer: 'rgb(0, 31, 36)', | ||
|
|
||
| secondary: 'rgb(74, 98, 103)', | ||
| onSecondary: 'rgb(255, 255, 255)', | ||
| secondaryContainer: 'rgb(205, 231, 236)', | ||
| onSecondaryContainer: 'rgb(5, 31, 35)', | ||
|
|
||
| tertiary: 'rgb(82, 94, 125)', | ||
| onTertiary: 'rgb(255, 255, 255)', | ||
| tertiaryContainer: 'rgb(218, 226, 255)', | ||
| onTertiaryContainer: 'rgb(14, 27, 55)', | ||
|
|
||
| error: 'rgb(186, 26, 26)', | ||
| onError: 'rgb(255, 255, 255)', | ||
| errorContainer: 'rgb(255, 218, 214)', | ||
| onErrorContainer: 'rgb(65, 0, 2)', | ||
|
|
||
| background: 'rgb(250, 253, 253)', | ||
| onBackground: 'rgb(25, 28, 29)', | ||
|
|
||
| surface: 'rgb(250, 253, 253)', | ||
| onSurface: 'rgb(25, 28, 29)', | ||
| surfaceVariant: 'rgb(219, 228, 230)', | ||
| onSurfaceVariant: 'rgb(63, 72, 74)', | ||
|
|
||
| outline: 'rgb(111, 121, 122)', | ||
| outlineVariant: 'rgb(191, 200, 202)', |
There was a problem hiding this comment.
We should be able to refer to these colors visually by clicking back to this PR from a vscode context menu near this code.
| const label: TextStyle = { | ||
| color: theme.colors.onPrimary, | ||
| marginVertical: 10, | ||
| marginHorizontal: 24, | ||
|
|
||
| fontFamily: Platform.select({ | ||
| web: 'Roboto, "Helvetica Neue", Helvetica, Arial, sans-serif', | ||
| ios: 'System', | ||
| default: 'sans-serif-medium', | ||
| }), | ||
|
|
||
| fontWeight: '500', | ||
| letterSpacing: 0.1, | ||
| lineHeight: 20, | ||
| fontSize: 14, | ||
| }; |
There was a problem hiding this comment.
Lots of this will be boilerplate for many components and should be moved to the theme. A good example of how this can be done is found here:
https://github.com/callstack/react-native-paper/blob/main/src/styles/themes/v3/tokens.tsx
There was a problem hiding this comment.
👍 Great offline discussion on RN paper too. I'm all for considering adopting that to avoid having to write too much code for things like Button, Text, icons, what props should a theme have, etc. Maybe their providers could be wrapped by ours if/where needed. Let's maybe shoot for a 1 day prototype on this next week
namick
changed the title
namick
force-pushed
the
PHC-4075-add-basic-wireframe-styling
branch
from
5d38319
to
6fd59da
Compare
| return { button, touchableOpacity, content, label }; | ||
| }; | ||
|
|
||
| declare module './BrandConfigProvider/types' { |
There was a problem hiding this comment.
I wonder if we could use TypeScript paths here to make this a bit easier. i.e. declare module '@style' {
There was a problem hiding this comment.
I spent some time trying to figure this out, but it sound like you already know the way! Awesome, will update in next PR
| fontSize: 14, | ||
| }; | ||
|
|
||
| return { button, touchableOpacity, content, label }; |
There was a problem hiding this comment.
Instead of declaring variables for each named style we could do something like:
return {
button: {
// ...
} as ViewStyle,
label: {
// ...
} as TextStyle,
There was a problem hiding this comment.
By casting, wouldn't we lose the type checking in the object itself? My understanding was to not prefer casting. For instance, this compiles just fine:
content = {
flexDirection: 'row',
alignItems: 'center',
justifyContent: 'center',
notValid: 'garbage',
} as ViewStyle;
There was a problem hiding this comment.
Great point! We shouldn't do that then.
|
🎉 This PR is included in version 1.1.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
github-actions
bot
added
the
released
Changes
Screenshots
Screen.Recording.2023-03-03.at.10.19.02.AM.mov