Skip to content

[FIX] Issue with should show splash for PearAI chat #269

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

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

charlwillia6
Copy link

@charlwillia6 charlwillia6 commented Feb 12, 2025

Description ✏️

When PearAI first loaded I saw this:
image

When we should be seeing this:
image

This PR fixes that issue. I only saw the splash on new chats, but not when PearAI first loads

Also fixed: Open button handling in inventoryPage.tsx. This was supposed to be a separate PR, but I accidentally pushed the commit to this branch. The supermaven open button stopped working, and I updated the open handling for the search and mem0 sidebar.

Checklist ✅

  • I have added screenshots (if UI changes are present).
  • I have done a self-review of my code.
  • I have manually tested my code (if applicable).

Important

Fixes splash screen display issue for PearAI chat by adjusting shouldShowSplash logic in gui.tsx.

  • Behavior:
    • Fixes splash screen display issue in gui.tsx by setting shouldShowSplash to isNewSession.
    • Removes useWebviewListener for highlightedCode to prevent premature splash screen hiding.
  • Misc:
    • Adds AUTO_COMPLETE_PROJECT.md with architecture details for auto-complete features.

This description was created by Ellipsis for d8385b7. It will automatically update as commits are pushed.

cwilliams and others added 27 commits October 27, 2024 20:28
The splash screen now only shows for new sessions, improving the user experience by removing redundant UI elements.  The splash screen's state is now directly derived from the application state, simplifying the code and removing the need for a separate state variable and listener.
@charlwillia6 charlwillia6 requested a review from a team as a code owner February 12, 2025 20:00
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to d8385b7 in 1 minute and 39 seconds

More details
  • Looked at 193 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. gui/src/pages/gui.tsx:181
  • Draft comment:
    Assigning shouldShowSplash directly to isNewSession removes the ability to toggle splash visibility after initial load. Is this change intentional?
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    The splash screen appears to be a welcome screen that should only show when there's no chat history. The old code allowed it to be toggled with setShouldShowSplash, but this seems unnecessary - the splash is meant to be a welcome screen only. The new code correctly ties it directly to isNewSession. The removed highlightedCode event handler that was toggling it off seems unrelated to the splash's purpose.
    Maybe there was a legitimate need to hide the splash screen in certain scenarios even during a new session that I'm not seeing? The removed event handler suggests there was some intentional behavior being removed.
    Looking at the full context, the splash is clearly meant to be a welcome screen showing keyboard shortcuts. There's no logical reason it should be toggleable - it should simply show when there's no history and hide when there is history. The old event handler appears to have been an unnecessary complexity.
    This comment should be deleted. The change simplifies the splash screen behavior to match its intended purpose as a welcome screen.
2. gui/src/pages/gui.tsx:346
  • Draft comment:
    The webview listener for "highlightedCode" that previously hid the splash has been removed. Confirm this is intended.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
    The removal of the highlightedCode listener appears to be intentional since it was replaced with a simpler, more direct approach - shouldShowSplash is now just tied to isNewSession. The old approach of using a listener to hide the splash seems more complex than necessary. The comment is asking for confirmation of an intentional change, which violates our rules about not asking for confirmations.
    The change could potentially break some functionality if the highlightedCode event was used for other purposes beyond just hiding the splash screen.
    The code shows a clear refactoring to simplify the splash screen logic. If there were other dependencies on this event, they would likely be visible in the codebase or cause build failures.
    This comment should be deleted because it's asking for confirmation of an intentional change, which violates our rules. The code changes show a clear refactoring that simplifies the logic.
3. AUTO_COMPLETE_PROJECT.md:12
  • Draft comment:
    Typo: "TabAutocomleteOptions" should be "TabAutocompleteOptions".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    While this is technically a typo, the review rules emphasize focusing on substantive issues that require code changes. This is a documentation file, and the typo is minor and doesn't affect functionality. The rules emphasize not making purely informative comments or obvious corrections.
    The typo could potentially cause confusion when developers look for this type in the codebase. It might make searching for references more difficult.
    While searchability is a concern, this is still a very minor documentation issue. The context makes it clear what is meant, and the actual type definition in code would be the source of truth.
    This comment should be deleted as it's a minor documentation typo that doesn't affect functionality and falls under the category of "obvious" or "unimportant" comments that the rules say to avoid.
4. gui/src/pages/gui.tsx:181
  • Draft comment:
    Consider removing the redundant variable 'shouldShowSplash'. Since it's set to 'isNewSession', you can directly use 'isNewSession' in rendering.
  • Reason this comment was not posted:
    Marked as duplicate.
5. gui/src/pages/gui.tsx:346
  • Draft comment:
    Removal of the 'highlightedCode' listener is intentional to rely solely on 'isNewSession'. Verify that this change aligns with desired UX since the splash now can’t be manually hidden.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_HKMpS8jIKPTnU60o


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

The AI tools in the inventory page are now invoked using the `ideMessenger` to communicate with the VSCode extension.  This improves the integration between the GUI and the extension, providing a more seamless user experience.  The `setTimeout` function is used for the autocomplete tool to ensure the command is invoked after the overlay closes.
@charlwillia6 charlwillia6 changed the title [FIX] Issue with should show splash for PearAI chat. [FIX] Issue with should show splash for PearAI chat Feb 12, 2025
@@ -178,7 +178,7 @@ function GUI() {
const [isAtBottom, setIsAtBottom] = useState<boolean>(false);
const state = useSelector((state: RootState) => state.state);
const isNewSession = state.history.length === 0;
const [shouldShowSplash, setShouldShowSplash] = useState(true);
const shouldShowSplash = isNewSession;
Copy link

@nang-dev nang-dev Feb 13, 2025

Choose a reason for hiding this comment

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

Yo charl, there are cases when this isnt right. like when you cmd+L code

Copy link
Author

Choose a reason for hiding this comment

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

@nang-dev So are you saying you don't want the splash to show when you CTRL+L?

Choose a reason for hiding this comment

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

Yes

Copy link
Author

Choose a reason for hiding this comment

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

Interesting. Okay. I'll work on that.

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