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

Traverse Rewrite #767

Merged
merged 15 commits into from
Jul 21, 2023
Merged

Conversation

joshua-davis-rose
Copy link
Contributor

PR Creator Checklist

Ensure you've checked the following before submitting your PR:

  • You've discussed making your changes with a member of the dev team per contributing rules in the README
  • Your changes are free of any lint errors
  • Your changes are free of any typescript errors
  • You've tested your changes

Summary

Please provide a summary of what your PR does

This covers a few tickets, including #736 and #708. I fixed the alpha indexes so they don't show after filtering if there are no items under that index. I also added some spacing above Subscriptions and added counts for Favorites and Subscriptions. These counts update based on the search.

This is a pretty major rewrite of Traverse. I've included Gluestack for the usage of HStack and VStack, but I also let the tool generate other Native Base components that we use so we can replace them over time.

Screenshots

If the UI has been changed, include screenshots.
We will not review your PR without screenshots if they are applicable.

My gif showing performance is too big to put here so I"ll share it in the contributors channel. Here's a screenshot showing the extra padding above Subscriptions along with the count.

Simulator Screenshot - iPhone 14 Pro Max - 2023-07-19 at 10 10 56

Test Plan

Please documemnt the steps required to test your PR

Go to traverse. Scroll. Filter. Favorite. Unfavorite. Make sure empty list messages are working.

@joshua-davis-rose joshua-davis-rose marked this pull request as ready for review July 20, 2023 00:06
@joshua-davis-rose
Copy link
Contributor Author

I've now tested this out on an iPad sim and my physical iPhone. I'm happy with how it is at this point so opening it up for full review. The big question is whether we move forward with Gluestack or not. It's...different, but it has similar performance as straight react and makes things a little easier to dev once you figure out its properties.

@theycallmebeez
Copy link

Before merging can you tweak the header size/style for Favorites and Subscribed to be similar to the section letter dividers?

Would also be nice to be able to sort the order of favorites. 😜

@joshua-davis-rose
Copy link
Contributor Author

joshua-davis-rose commented Jul 20, 2023

@theycallmebeez If there isn't an issue for the sorting can you add one? That's not trivial to add this go around. I'll talk with the other devs about the header sizing. I'm no designer.

@theycallmebeez
Copy link

@theycallmebeez If there isn't an issue for the sorting can you add one? That's not trivial to add this go around. I'll talk with the other devs about the header sizing. I'm no designer.

all done - #772

Copy link
Collaborator

@sgriff96 sgriff96 left a comment

Choose a reason for hiding this comment

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

LGTM, definitely seems more performant

@gkasdorf gkasdorf merged commit 2472749 into Memmy-App:main Jul 21, 2023
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.

4 participants