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

Points History and Rewards tab bar styling + Rewards navigation animations #46

Merged
merged 13 commits into from
Mar 26, 2020

Conversation

wangannie
Copy link
Member

@wangannie wangannie commented Mar 25, 2020

What's new in this PR

Overall, made Rewards and Point History look and feel smoother + various other cleanup.

  • Made a new CircleIcon component to standardize all the repeated circle icons throughout the app
  • Fully styled transaction cards in the points history screen and updated copy to match Figma
  • Updated transaction card content:
    • Fixed date formatting
    • Renamed the Airtable call to match the new column name 'Points Earned'
    • Added sale total
  • Fixed up styling details in the tab view and cleaned up the Rewards heading bar
  • Rearranged drawer navigator so the Rewards page slides up as a modal to match the Figma prototypes
  • Made 'Points History' in the drawer correctly link to Points History (I think the way I did this was a bit jank so I'd really appreciate some more opinions 😅 )

Relevant Links

Online sources

The reasoning for adding RootStack with Rewards as a modal came from this.

https://stackoverflow.com/questions/44248403/passing-props-with-screen-option-in-drawernavigator

Sample modal interaction Expo Snack

Related PRs

Continuing to finalize and clean up details from #42 and #30

How to review

  • Nitpick my code! I wasn't really sure if it was a good idea to put CircleIconContainer in BaseComponents, but I didnt' want to have to make a another styled just for that... thoughts?
  • Look for styling details I might have missed
  • Click around and test the navigation interactions to see if anything breaks with the new RootStack

Next steps

  • Make different kinds of points history cards (not just transactions) to match the designs, including reward redeemed, reward unlocked etc.
  • Currently, if you switch to the 'My Rewards' tab from opening 'Points History' from the drawer, it will open back to 'My Rewards' if you hit 'Points History' from the drawer again. I decided not to worry about this now based on @anniero98 's recent findings about the updates coming with react-navigation v5.
  • Try to figure out how to smooth the transition closing Rewards after it's been opened from the drawer...it would be nice if it slid down but at the moment it just disappears.
  • Consider how we may be able to condense the new CircleIcon with NavButton from Navigation Button restyling + Misc. style fixes #42
  • Cool scrolling animation where 'Healthy Rewards' shrinks and centers upon scrolling :^)
  • Look into Section List for date dividers for points history

Tests Performed, Edge Cases

  • Tested on iPhone 11 Pro Max, and iPhone 8 and it the transactions cards, tab bars and heading seem to be responsive

Screenshots

🥳Rewards finally slides upward!
CC: @anniero98 @wangannie

@wangannie wangannie self-assigned this Mar 25, 2020
Comment on lines +27 to 38
export const CircleIconContainer = styled.View`
display: flex;
align-items: center;
justify-content: center;
height: 40px;
width: 40px;
padding: 8px;
border-radius: 20px;
background-color: ${props => props.color || Colors.lighterGreen};
`;

export const ButtonContainer = styled.TouchableOpacity``;
Copy link
Contributor

Choose a reason for hiding this comment

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

style thing? i think a lot of our other stuff has the icon vertically centered so maybe we should change this to match that, but @12aschen should chime in

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean—vertically centered to what? Lines 29 & 30 should make it centered.

Comment on lines +14 to +18
<CircleIcon
icon="star"
iconColor={Colors.primaryGreen}
circleColor={Colors.lightest}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

ty for the fix here, it was so jank before LOL

Copy link
Contributor

@kennethlien kennethlien left a comment

Choose a reason for hiding this comment

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

Just added a couple comments, looks very cool! B)

Copy link
Collaborator

@annieyro annieyro left a comment

Choose a reason for hiding this comment

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

Did not try on simulator because I've been working on refactor for awhile 😅 Small nits but code looks good!

@@ -31,18 +28,21 @@ StoresStack.navigationOptions = {
drawerLabel: 'Stores'
};

export const RewardsStack = createStackNavigator(
export const RootStack = createStackNavigator(
Copy link
Collaborator

Choose a reason for hiding this comment

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

very cool !!

@@ -141,18 +144,17 @@ export default class RewardsScreen extends React.Component {
return (
<Container>
<TopTab>
<BackButton onPress={() => this.props.navigation.navigate('Stores')}>
<BackButton onPress={() => this.props.navigation.goBack()}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this work even if you access the Rewards screen by navigating there from the drawer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep! I changed it to goBack because it would reload the entire map when I closed it from the drawer and that got really slow.

<Overline style={{ marginTop: 24, marginBottom: 12 }}>
Reward Progress
</Overline>
<Title style={{ marginBottom: 2 }}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Figma has the first number in BigTitle 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

I think Kenneth and I agreed on abandoning that when the alignment of 2 different line heights was getting really annoying 😶

@wangannie wangannie merged commit 9f3b344 into master Mar 26, 2020
@wangannie wangannie deleted the AnnieW/points_history_styling branch March 26, 2020 07:19
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.

3 participants