Skip to content

Comments

feat: add Favorites tab to bottom navigation#11686

Merged
MounirDhahri merged 4 commits intomainfrom
feature/favorites-tab
Mar 18, 2025
Merged

feat: add Favorites tab to bottom navigation#11686
MounirDhahri merged 4 commits intomainfrom
feature/favorites-tab

Conversation

@nickskalkin
Copy link
Contributor

@nickskalkin nickskalkin commented Mar 14, 2025

  • Added new feature flag AREnableFavoritesTab (default: false)
  • Added heart icon for Favorites tab
  • Added conditional rendering based on feature flag
  • Placed tab between Inbox and Profile tabs
  • Added analytics tracking

🤖 Generated with Claude Code

This PR resolves ONYX-1599

Description

This PR adds the boilerplate code requires for the new favorites hub.

Screen.Recording.2025-03-14.at.12.39.27.mov

PR Checklist

  • I have tested my changes on iOS and Android.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

  • add favorites tab boilerplate code - nikita, mounir

iOS user-facing changes

Android user-facing changes

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

nickskalkin and others added 2 commits March 14, 2025 12:42
- Added new feature flag AREnableFavoritesTab (default: false)
- Added heart icon for Favorites tab
- Added conditional rendering based on feature flag
- Placed tab between Inbox and Profile tabs
- Added analytics tracking

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Mounir Dhahri <soussouheros@gmail.com>
Co-authored-by: Nikita Skalkin <nikskalkin@gmail.com>
@MounirDhahri MounirDhahri force-pushed the feature/favorites-tab branch from e16b6b3 to 94d66a4 Compare March 14, 2025 11:42
@MounirDhahri MounirDhahri marked this pull request as ready for review March 14, 2025 11:42
@ArtsyOpenSource
Copy link
Contributor

ArtsyOpenSource commented Mar 14, 2025

This PR contains the following changes:

  • Cross-platform user-facing changes (add favorites tab boilerplate code - nikita, mounir - nickskalkin)

Generated by 🚫 dangerJS against aceb0b5

anandaroop
anandaroop previously approved these changes Mar 14, 2025
Copy link
Member

@anandaroop anandaroop left a comment

Choose a reason for hiding this comment

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

Nice 😎

The 2nd commit mentions following up on Claude. What kind of follow-up was necessary? Did it involve e.g a substantive change to the SVG code?

},
favorites: {
route: "/favorites",
analyticsDescription: OwnerType.favorites,
Copy link
Member

Choose a reason for hiding this comment

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

question (non-blocking): Noticing the different owner type here vs above, I assume it's intentional?

Copy link
Member

Choose a reason for hiding this comment

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

no that was a mistake - I will address it

dariakoko
dariakoko previously approved these changes Mar 15, 2025
Copy link
Contributor

@dariakoko dariakoko left a comment

Choose a reason for hiding this comment

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

Wow that's impressive. How much of it was generated by Claude?

olerichter00
olerichter00 previously approved these changes Mar 17, 2025
return (
<ProvideScreenTracking
info={{
context_screen: Schema.PageNames.SavesAndFollows,
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Is it intentional that this does not come from Cohesion?

Copy link
Member

Choose a reason for hiding this comment

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

This file probably precedes all of us and is maybe from a time before cohesion existed 😄. We didn't make changes to it because it's going to get removed once we roll out.

@MounirDhahri
Copy link
Member

The first commit was all Claude.
A few things Claude struggled with:

  • Figuring out that the route is already there and interpret it with a feature flag. That's probably expected because we have no other example of it in the app. I think Claude would have done great a job creating a new screen - that would have been less complicated than a tab
  • It started creating the feature in the native side at AROptions instead of features.ts.

That being said, it quickly adjusted to our prompts and it even generated a heart icon that was still within the bottom tab (didn't require any pixel adjustments).

},
favorites: {
active: (
<Path
Copy link
Contributor

Choose a reason for hiding this comment

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

This icon should be on Icons repo

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely - we have a separate ticket to address the Icons on the navbar

@MounirDhahri MounirDhahri merged commit f52c8b6 into main Mar 18, 2025
6 of 7 checks passed
@MounirDhahri MounirDhahri deleted the feature/favorites-tab branch March 18, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants