favorite folders followup #8570
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR simplifies the favorites folder picker implementation and fixes checkbox styling issues, while addressing rendering behavior of the favorites section.
- Removed redundant filtering in
/packages/twenty-front/src/modules/favorites/components/CurrentWorkspaceMemberFavoritesFolders.tsxas it's now handled byusePrefetchedFavoritesData - Removed custom
FavoriteFolderMenuItemMultiSelectcomponent in favor of usingMenuItemMultiSelectdirectly from twenty-ui - Removed custom padding from IconPlus in
/packages/twenty-front/src/modules/favorites/favorite-folder-picker/components/FavoriteFolderPickerFooter.tsx - Issue If there are no favorites and a folder is added, favorite section doesnt render until it has atleast one favorite #8569 remains open due to feature flag constraints preventing folders from rendering without favorites
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
ehconitin
changed the title
...s/twenty-front/src/modules/favorites/favorite-folder-picker/hooks/useFavoriteFolderPicker.ts
Outdated
Show resolved
Hide resolved
...s/twenty-front/src/modules/favorites/favorite-folder-picker/hooks/useFavoriteFolderPicker.ts
Outdated
Show resolved
Hide resolved
charlesBochet
approved these changes
Nov 19, 2024
ehconitin
commented
Nov 19, 2024
|
|
||
| const shouldDisplayFavorites = isFavoriteFolderEnabled | ||
| ? shouldDisplayFavoritesWithFeatureFlagEnabled | ||
| : shouldDisplayFavoritesWithoutFeatureFlagEnabled; | ||
|
|
There was a problem hiding this comment.
@charlesBochet need confirmation on this, rest everything looks good to me!
|
Thanks @ehconitin for your contribution! |
BKM14
pushed a commit
to BKM14/twenty
that referenced
this pull request
Nov 20, 2024
Something changed, which affected the Favorite folder picker checkbox styles -- fixed it! Cleaned up code in `CurrentWorkspaceMemberFavoritesFolders` - removed redundant filtering since favorites are already filtered in `usePrefetchedFavoritesData`. Regarding issue twentyhq#8569 - I am not sure what to do in this case. Since Folders data is gated by a feature flag, we can't use it in `CurrentWorkspaceMemberFavoritesFolders` to ensure the favorite section renders with empty folders. Currently, the section only appears when at least one favorite exists - may be leave this section open at all times or fix this bug after removal of the feature flag? --------- Co-authored-by: Nitin Koche <[email protected]> Co-authored-by: Charles Bochet <[email protected]>
This was referenced Nov 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Something changed, which affected the Favorite folder picker checkbox styles -- fixed it!
Cleaned up code in
CurrentWorkspaceMemberFavoritesFolders- removed redundant filtering since favorites are already filtered inusePrefetchedFavoritesData.Regarding issue #8569 - I am not sure what to do in this case. Since Folders data is gated by a feature flag, we can't use it in
CurrentWorkspaceMemberFavoritesFoldersto ensure the favorite section renders with empty folders. Currently, the section only appears when at least one favorite exists - may be leave this section open at all times or fix this bug after removal of the feature flag?