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

H-1643: Add draft indicators to browser plugin jobs, add tooltips #4168

Merged
merged 4 commits into from
Mar 14, 2024

Conversation

CiaranMn
Copy link
Member

🌟 What is the purpose of this PR?

Breaks this commit out of #3953, which is stuck.

UX improvements to the browser plugin:

  1. Add a feather icon next to drafts
  2. Add tooltips to indicators so it's more obvious what they do

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

You might have to run the browser plugina via yarn build && yarn start rather than yarn dev due to some bundling issues which will be fixed by changes to @blockprotocol/core made in blockprotocol/blockprotocol#1373, but that is blocked by converting a bunch of stuff in this repo to ESM.

❓ How to test this?

  1. Run the browser plugin, run a job that creates drafts, check indicators

benwerner01
benwerner01 previously approved these changes Mar 14, 2024
Copy link
Contributor

@benwerner01 benwerner01 left a comment

Choose a reason for hiding this comment

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

Thanks Ciaran, this looks good to me and is working as expected. Had one thought whilst testing it out, but doesn't block merging.

@@ -114,47 +116,82 @@ const InferenceRequestContainer = ({
{request.status === "pending" ? (
<Stack alignItems="center" direction="row">
{cancellationRequested ? (
<Tooltip title="Cancellation pending...">
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should rethink the positioning of the tooltips here, it's making it slightly harder to hover from the right icon to the left icon because of the placement:

Screen.Recording.2024-03-14.at.15.02.22.mov

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved in a16e59e

@CiaranMn CiaranMn added this pull request to the merge queue Mar 14, 2024
Merged via the queue into main with commit 4aba379 Mar 14, 2024
27 of 28 checks passed
@CiaranMn CiaranMn deleted the cm/fix-browser-plugin-indicators branch March 14, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants