Skip to content

Conversation

@danielavornic
Copy link
Contributor

related to #73

This PR implements a favorites system that allows users to favorite/unfavorite feed links links.

Features

  • Added favorites storage in IndexedDB with a dedicated table
  • Added favorite/unfavorite functionality to feed items
  • Created a favorites list in a separate tab-like layout from existing "Feeds"
  • Added star button to feed items for favoriting on hover
  • Added support for favorites export/import along with the other feeds

Implementation

  • Created Favorite model and favoritesService
  • Implemented favorites management through a useFavorites hook
  • Updated FeedItem component to support favoriting
  • Added favorites export/import to the data transfer system that already existed

Screenshots

Feeds view

image
For non-favorite links, the unfilled star appears on hover.

Favorites view

image
I hope it's ok that I show only the "X" button for unfavorite here and not the stars, given that all the links here would have a star on the left. Let me know if a consistent view for link items would be preferable, and also whether a confirmation dialog for favorite removal is too inconvenient.

@strdr4605 strdr4605 requested review from Copilot and strdr4605 April 15, 2025 06:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a favorites functionality that lets users mark feed items as favorites, persist them in IndexedDB, and manage them via UI components and data export/import features.

  • Added migration and updates to IndexedDB functions for a new Favorites table.
  • Created favorites service functions and a custom hook (useFavorites) for managing favorites state and actions.
  • Updated UI components (FeedItem, FavoritesList, ExportImportForm, etc.) and pages to integrate the favorites feature.

Reviewed Changes

Copilot reviewed 20 out of 23 changed files in this pull request and generated no comments.

Show a summary per file
File Description
services/migrations/V2_add_favorites.ts Created a migration to add the favorites table in IndexedDB.
services/indexeddbCRUD.ts Updated table creation functions to work with the new TableSchema interface.
services/favoritesService.ts Added functions for getting, adding, and removing favorites using IndexedDB.
services/exportService.ts Modified the export/import functions to include favorites data along with feeds.
pages/index.tsx Integrated a tabbed view to support both feeds and favorites, including routing updates.
models/index.ts, models/TransferFeed.ts, Updated models to include the Favorite model and adjust transfer data structure.
models/Favorite.ts New Favorite model definition.
hooks/useFavorites.ts & hooks/index.ts Introduced a custom hook to manage the favorite state and handle favorite actions.
components/* Added/updated multiple components (FeedItem, FavoritesList, ExportImportForm, etc.) to enable favorites UI.
Files not reviewed (3)
  • components/FavoritesList/FavoritesList.module.css: Language not supported
  • components/FeedItem/FeedItem.module.css: Language not supported
  • package.json: Language not supported

@strdr4605
Copy link
Member

I have migrated the project to [email protected], so please rebase. Also, please run yarn format. and commit the changes😊

@danielavornic
Copy link
Contributor Author

I know it's not a good practice, but I had to use git push --force-with-lease on this branch to clean up the commit history. The rebase attempt left an orphaned commit on the remote and this brings the remote branch back in sync with the "correct" local history.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a favorites functionality to the feed system by implementing a dedicated favorites model, services, and UI components. Key changes include:

  • Adding a Favorite model and updating data transfer structures to include favorites.
  • Implementing a useFavorites hook and integrating favorite toggling into FeedItem, VisitedItemsList, and NewItemsList components.
  • Updating export/import functionality and workflows to support the new favorites data.

Reviewed Changes

Copilot reviewed 27 out of 31 changed files in this pull request and generated no comments.

Show a summary per file
File Description
models/index.ts Exports the new Favorite model.
models/TransferFeed.ts Adds favorites to the transfer data interface.
models/Favorite.ts Introduces the Favorite model with necessary properties.
hooks/useFavorites.ts Implements a hook to manage favorites state and toggling.
components/VisitedItemsList/VisitedItemsList.tsx Integrates the useFavorites hook into visited items display.
components/NewItemsList/NewItemsList.tsx Integrates the useFavorites hook into new items display.
components/FeedItem/FeedItem.tsx Displays favorite button with toggling functionality.
components/FavoritesList/FavoritesList.tsx Provides a UI component for viewing and managing favorites.
components/ExportImportForm/ExportImportForm.tsx Updates export/import functions to handle favorites data.
.github/workflows/* Updates to workflow versions and Node.js version adjustments.
Files not reviewed (4)
  • .oxlintrc.json: Language not supported
  • components/FavoritesList/FavoritesList.module.css: Language not supported
  • components/FeedItem/FeedItem.module.css: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (2)

models/Favorite.ts:4

  • [nitpick] The Favorite interface extends RSSParser.Item which may already include a 'link' property. Consider unifying the identifier field (either use 'url' consistently or rely on 'link') to avoid potential confusion.
url: string;

components/FavoritesList/FavoritesList.tsx:41

  • [nitpick] The component uses 'favorite.url' for the key while removal functions refer to 'favorite.link'. Consider using a consistent identifier property for favorites across the codebase.
<li key={favorite.url} className={Styles.item}>

@strdr4605
Copy link
Member

I know it's not a good practice, but I had to use git push --force-with-lease on this branch to clean up the commit history. The rebase attempt left an orphaned commit on the remote and this brings the remote branch back in sync with the "correct" local history.

Who says "that git push --force is not a good practice", does not know git well enough, and is afraid to mess it up.
I do force pushes multiple times a day. More often git push -f (for force), because I am too lazy to type --force-with-lease😅.
But I have some git aliases for git push --force-with-lease when I work with [stacked PRs]
(https://blog.logrocket.com/using-stacked-pull-requests-in-github/).


Why I prefer rebase and push force over merge

Copy link
Member

@strdr4605 strdr4605 left a comment

Choose a reason for hiding this comment

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

Now let's fix commits in this PR

This is the git state on my local machine (fetched from this PR)
image

We don't need these commits, are they are already on cofeed-19:master. In my case, origin/master, in your case upstream/master` (probably)

image

You should run git rebase --interactive upstream/master (assuming upstream is cofeed-19)

When the git interactive menu opens, remove any redundant commits

image

and close the menu :wq.

Probably you will have to fix this conflict

image

Pick:
image

Then run yarn, git add -A, git rebase --continue.

And finally git push --force. If you have issues or questions, let me know.

@danielavornic
Copy link
Contributor Author

I know it's not a good practice, but I had to use git push --force-with-lease on this branch to clean up the commit history. The rebase attempt left an orphaned commit on the remote and this brings the remote branch back in sync with the "correct" local history.

Who says "that git push --force is not a good practice", does not know git well enough, and is afraid to mess it up. I do force pushes multiple times a day. More often git push -f (for force), because I am too lazy to type --force-with-lease😅. But I have some git aliases for git push --force-with-lease when I work with [stacked PRs] (https://blog.logrocket.com/using-stacked-pull-requests-in-github/).

Why I prefer rebase and push force over merge

Got it, thank you sm 😅 Was scared i messed something up, I'm not experienced with rebase that much.

@danielavornic danielavornic requested a review from strdr4605 April 18, 2025 08:28
@strdr4605 strdr4605 merged commit 4be4567 into cofeed-19:master Apr 18, 2025
2 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 18, 2025
* feat: implement add to favorites functionality

- updated database version to 2 and added FavoriteTable schema
- introduced FavoritesList component
- added functionality to add, fetch and check favorites
- refactored components to integrate favorites feature
- updated main page to include a tab for favorites and feeds

* feat: add remove favorite functionality

* refactor: create and integrate useFavorites hook

* feat: update export/import to handle favorites

* refactor: update TableSchema type and rename SiteFeed table

* style: update remove button in FavoritesList

* fix: update Favorite model source feed url

* refactor: update favorites handling in components and useFavorites hook

* refactor: remove unused feed prop from FeedItem 4be4567
@strdr4605
Copy link
Member

Thanks for contribution😊

@all-contributors please add @danielavornic for code

@allcontributors
Copy link
Contributor

@strdr4605

I've put up a pull request to add @danielavornic! 🎉

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.

2 participants