Skip to content
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

Implement Button Functionality for Launching Pieces OS #52

Merged
merged 6 commits into from
Dec 24, 2023

Conversation

agrim0312
Copy link
Contributor

@agrim0312 agrim0312 commented Dec 20, 2023

Description
Implement Button Functionality for Launching Pieces OS
This PR fixed #41
Problem Statement:
Currently, when Pieces OS is not connected, the UI lacks a functional aspect to initiate the connection. To improve user interaction, we want to add button functionality to the "Not Connected" button, allowing users to launch Pieces OS by clicking it.

Expected Outcome:
Users will be able to launch Pieces OS by clicking the "Not Connected" button, enhancing the user experience and providing a straightforward way to establish a connection.

Copy link
Contributor

@shivay-at-pieces shivay-at-pieces left a comment

Choose a reason for hiding this comment

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

Hey @agrim0312 great work here. Do you mind also addressing the issue in #46
At the moment, we don't have an error boundary for Pieces OS not running. So if we can also take care of that, it should prevent from the error popping up.

@agrim0312
Copy link
Contributor Author

@shivay-at-pieces As error-boundary is used only for rendering error and these errors were overthrown by network error So I just created a div that will render when the piecesOS is not running in the background. I tested it in the build and it worked fine. I hope that this was the issue that you were refering to . If I missunderstood the issue please pardon me.
issue fixed #46
description - add an error boundary, and show in the project itself that Pieces OS isn't running in the background right now.
image

@shivay-at-pieces
Copy link
Contributor

shivay-at-pieces commented Dec 20, 2023

At the moment, it shows that you are connected to Pieces in the badge even though Pieces isn't running in the background. You might want to fix that state.
image

@agrim0312
Copy link
Contributor Author

sure I'll fix it

@agrim0312
Copy link
Contributor Author

@shivay-at-pieces Fixed the text. You can review it now.

@shivay-at-pieces
Copy link
Contributor

Hey @agrim0312 there is still some work remaining on how we handle the button state when Pieces OS is connected and disconnected, as you can see in this video. You can see that the state of the button doesn't change. and we have to refresh to see the state change when Pieces OS is connected/disconnected

Screen.Recording.2023-12-21.at.2.32.03.AM.mov

@agrim0312
Copy link
Contributor Author

sure working on it

@agrim0312
Copy link
Contributor Author

@shivay-at-pieces I used prop drilling method to manage the state of button. I hope you were looking for this.

@jwafu
Copy link
Contributor

jwafu commented Dec 24, 2023

this is a much better improvement here and will at least get users up and running when they are not able to see snippets in the viewer, as well as with the new copilot chat. We we could potentially improve here by adding a loading state while a connection is being made that way we avoid ever hitting an error. @agrim0312 or @shivay-at-pieces if you want to get an issue created with details on an improvement and link it back to this pr that would be great!

Copy link
Contributor

@jwafu jwafu left a comment

Choose a reason for hiding this comment

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

looking good! be sure to get some general comments in a few more locations in future PRs 👍

@jwafu jwafu merged commit 3c8b111 into pieces-app:main Dec 24, 2023
1 check passed
@agrim0312
Copy link
Contributor Author

sure 👍 @jordan-pieces

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.

Implement Button Functionality for Launching Pieces OS
3 participants