Skip to content

feat(client): added bookmark feature on preview app#1296

Merged
johbaxter merged 19 commits intodevfrom
29-Brookmark-from-preview-app
Jul 17, 2025
Merged

feat(client): added bookmark feature on preview app#1296
johbaxter merged 19 commits intodevfrom
29-Brookmark-from-preview-app

Conversation

@Paulson-Robert
Copy link
Copy Markdown
Contributor

@Paulson-Robert Paulson-Robert commented Jun 13, 2025

Description

I will be able to bookmark the app in that preview window so that if I leave and go to the All Apps page (or anywhere else) I can easily find that app again.

Changes Made

Placed a bookmark icon in the app preview page and integrated existing bookmark reactor into it.

How to Test

It has a Backend PR associated with this -Link

  • Bookmark an app in the preview page
  • You should be able to see the changes reflecting in the pp catalog page and vice versa
App.bookmark.webm

@Paulson-Robert Paulson-Robert requested a review from johbaxter June 13, 2025 06:41
@Paulson-Robert Paulson-Robert self-assigned this Jun 13, 2025
@Paulson-Robert Paulson-Robert requested a review from a team as a code owner June 13, 2025 06:41
@github-actions
Copy link
Copy Markdown

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

feat(client): added bookmark feature on preview app


User description

Description

Changes Made

How to Test

  1. Steps to reproduce/test the behavior
  2. Expected outcomes

Notes


PR Type

Enhancement


Description

  • Add bookmark state and handler

  • Initialize bookmark from workspace metadata

  • Call monolithStore.setProjectFavorite API

  • Insert bookmark icon button in header


Changes walkthrough 📝

Relevant files
Enhancement
AppPage.tsx
Implement project bookmark functionality                                 

packages/client/src/pages/app/AppPage.tsx

  • Imported Bookmark and BookmarkBorderOutlined icons
  • Added bookmarked state variable and handleBookmark function
  • Initialized bookmark state from
    loadedWorkspace.metadata.project_favorite
  • Inserted bookmark IconButton with toggle in app header
  • +44/-2   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /review

    @Paulson-Robert
    Copy link
    Copy Markdown
    Contributor Author

    Paulson-Robert commented Jun 13, 2025

    @reviewers @johbaxter
    BE changes dependency PR : SEMOSS/Semoss#899

    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Stale state reference

    The notification message in handleBookmark uses the bookmarked state instead of the status argument, which may result in incorrect messaging. Use status to determine whether the project was bookmarked or unbookmarked.

    const handleBookmark = (status: boolean) => {
        setBookmarked(status);
        monolithStore
            .setProjectFavorite(appId, status)
            .then(() => {
                notification.add({
                    color: 'success',
                    message: `Project ${
                        bookmarked ? 'unbookmarked' : 'bookmarked'
                    }`,
                });
                return;
    Error handling

    Throwing an error inside the catch block can lead to unhandled promise rejections and crash the app. Consider showing an error notification or handling the failure more gracefully instead of throwing.

    .catch((err) => {
        // throw error if promise doesn't fulfill
        throw Error(err);
    });

    @QodoAI-Agent
    Copy link
    Copy Markdown

    QodoAI-Agent commented Jun 13, 2025

    PR Code Suggestions ✨

    Latest suggestions up to dffdc87

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use status for correct notification

    The success message uses the old bookmarked state which is stale inside the promise
    callback; use the status parameter to compute the message instead. This ensures the
    notification text accurately reflects the new state.

    packages/client/src/pages/app/ViewAppPage.tsx [54-59]

     notification.add({
         color: 'success',
    -    message: `Project ${
    -        bookmarked ? 'unbookmarked' : 'bookmarked'
    -    }`,
    +    message: status ? 'Project bookmarked' : 'Project unbookmarked',
     });
    Suggestion importance[1-10]: 8

    __

    Why: The callback uses stale bookmarked state for the notification message; using the status parameter ensures the message accurately reflects the new bookmark state.

    Medium
    General
    Show error notification on failure

    Throwing an error inside the catch will crash the app; instead show an error
    notification and revert the UI state if needed. This provides better user feedback
    on failure.

    packages/client/src/pages/app/ViewAppPage.tsx [62-65]

     .catch((err) => {
    -    // throw error if promise doesn't fulfill
    -    throw Error(err);
    +    setBookmarked(!status);
    +    notification.add({
    +        color: 'error',
    +        message: `Failed to ${status ? 'bookmark' : 'unbookmark'} project`,
    +    });
     });
    Suggestion importance[1-10]: 7

    __

    Why: Throwing an error here would crash the app; showing an error notification and reverting the UI provides better user feedback and maintains consistency.

    Medium
    Memoize bookmark handler

    Wrapping handleBookmark in useCallback prevents unnecessary re-creations on each
    render and ensures stable references for event handlers.

    packages/client/src/pages/app/ViewAppPage.tsx [49-66]

    -const handleBookmark = (status: boolean) => {
    +const handleBookmark = useCallback((status: boolean) => {
         setBookmarked(status);
         monolithStore
             .setProjectFavorite(appId, status)
             .then(...)
             .catch(...);
    -};
    +}, [monolithStore, appId]);
    Suggestion importance[1-10]: 4

    __

    Why: Wrapping handleBookmark in useCallback prevents unnecessary recreations but offers only a minor performance benefit in this context.

    Low

    Previous suggestions

    Suggestions up to commit 8d58436
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Defer state update until success

    Move the setBookmarked call into the .then callback to apply the update only on
    success, use the status parameter for the notification text, and replace the thrown
    error with a user-facing error notification.

    packages/client/src/pages/app/AppPage.tsx [62-79]

     const handleBookmark = (status: boolean) => {
    -    setBookmarked(status);
         monolithStore
             .setProjectFavorite(appId, status)
             .then(() => {
    +            setBookmarked(status);
                 notification.add({
                     color: 'success',
    -                message: `Project ${
    -                    bookmarked ? 'unbookmarked' : 'bookmarked'
    -                }`,
    +                message: `Project ${status ? 'bookmarked' : 'unbookmarked'}`,
                 });
    -            return;
             })
    -        .catch((err) => {
    -            // throw error if promise doesn't fulfill
    -            throw Error(err);
    +        .catch(() => {
    +            notification.add({
    +                color: 'error',
    +                message: 'Failed to update bookmark',
    +            });
             });
     };
    Suggestion importance[1-10]: 7

    __

    Why: Ensures bookmarked state is only updated after a successful API response and uses the status parameter for correct notification text, while providing a user-facing error message instead of throwing.

    Medium

    @Paulson-Robert Paulson-Robert linked an issue Jun 13, 2025 that may be closed by this pull request
    1 task
    @Paulson-Robert
    Copy link
    Copy Markdown
    Contributor Author

    @johbaxter johbaxter merged commit 8be6e54 into dev Jul 17, 2025
    3 checks passed
    @johbaxter johbaxter deleted the 29-Brookmark-from-preview-app branch July 17, 2025 18:22
    @github-actions
    Copy link
    Copy Markdown

    @CodiumAI-Agent /update_changelog

    @QodoAI-Agent
    Copy link
    Copy Markdown

    Changelog updates: 🔄

    2025-07-17

    Added

    • Bookmark toggle on app preview page to mark/unmark favorites and sync with catalog listings.

    to commit the new content to the CHANGELOG.md file, please type:
    '/update_changelog --pr_update_changelog.push_changelog_changes=true'

    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.

    [FEAT] Allow Bookmarking on Preview of App

    3 participants