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

Capture screen like similar to Claude #1604

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

Conversation

harisyammnv
Copy link
Contributor

I tried to get a simpler version of Screen capture similar to the one in Claude here
@nsarrazin : your comments would be appreciated here

Copy link
Collaborator

@nsarrazin nsarrazin left a comment

Choose a reason for hiding this comment

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

Hi @harisyammnv I merged the latest changes to the tool UI to your branch and updated a few things to make it work there.

The icon now shows whenever we support uploading images in the file upload MIME types. So tools that accept images and multimodal models are both supported.

cc @gary149 for review, if we want to add this in prod or not

@nsarrazin nsarrazin added the front This issue is related to the front-end of the app. label Jan 6, 2025
@nsarrazin nsarrazin requested a review from gary149 January 7, 2025 07:57
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capitalized: IconScreenshot.svelte

Comment on lines +5 to +19
<svg
class={classNames}
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
focusable="false"
role="img"
width="1em"
height="1em"
fill="currentColor"
viewBox="0 0 24 24"
>
<path
d="M15 16h4v-4h-1.5v2.5H15zM5 10h1.5V7.5H9V6H5zm3 11v-2H4q-.825 0-1.412-.587T2 17V5q0-.825.588-1.412T4 3h16q.825 0 1.413.588T22 5v12q0 .825-.587 1.413T20 19h-4v2zm-4-4h16V5H4zm0 0V5z"
/>
</svg>
Copy link
Collaborator

@gary149 gary149 Jan 7, 2025

Choose a reason for hiding this comment

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

Custom icon that matches the others better:

Suggested change
<svg
class={classNames}
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
focusable="false"
role="img"
width="1em"
height="1em"
fill="currentColor"
viewBox="0 0 24 24"
>
<path
d="M15 16h4v-4h-1.5v2.5H15zM5 10h1.5V7.5H9V6H5zm3 11v-2H4q-.825 0-1.412-.587T2 17V5q0-.825.588-1.412T4 3h16q.825 0 1.413.588T22 5v12q0 .825-.587 1.413T20 19h-4v2zm-4-4h16V5H4zm0 0V5z"
/>
</svg>
<svg
class={classNames}
xmlns="http://www.w3.org/2000/svg"
aria-hidden="true"
focusable="false"
role="img"
width="1em"
height="1em"
fill="currentColor"
viewBox="0 0 32 32"
><path
fill-rule="evenodd"
clip-rule="evenodd"
d="M5.98 6.01h20.04v16.1H5.98V6.02Zm-2.1 0c0-1.16.94-2.1 2.1-2.1h20.04c1.16 0 2.1.94 2.1 2.1v16.1a2.1 2.1 0 0 1-2.1 2.11H5.98a2.1 2.1 0 0 1-2.1-2.1V6.02Zm5.7 1.65a2.1 2.1 0 0 0-2.1 2.1v2.61a1.05 1.05 0 0 0 2.1 0v-2.6h2.96a1.05 1.05 0 1 0 0-2.11H9.58ZM24.41 18.4a2.1 2.1 0 0 1-2.1 2.1h-2.95a1.05 1.05 0 1 1 0-2.1h2.95v-2.61a1.05 1.05 0 0 1 2.1 0v2.6ZM10.1 25.9a1.05 1.05 0 1 0 0 2.1H22.3a1.05 1.05 0 1 0 0-2.1H10.1Z"
/>
</svg>
image

@@ -250,6 +252,27 @@
</label>
</HoverTooltip>
</form>
{#if mimeTypes.includes("image/*")}
<HoverTooltip label="Capture screenshot" position="top">
Copy link
Collaborator

@gary149 gary149 Jan 7, 2025

Choose a reason for hiding this comment

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

positioning could be better:
image

Comment on lines +417 to +418
<div class="flex w-full items-center gap-2">
<form
Copy link
Collaborator

@gary149 gary149 Jan 7, 2025

Choose a reason for hiding this comment

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

didn't test extensively but I would prefer not changing the elements wrapping here.

console.error("Error capturing screenshot:", error);
throw error;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be safer to stop all tracks in a finally block

	} finally {
		if (stream) {
			stream.getTracks().forEach((track) => track.stop());
		}
	}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front This issue is related to the front-end of the app.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants