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

Update Assistant-related icons #210

Merged
merged 5 commits into from
Jun 6, 2024
Merged

Update Assistant-related icons #210

merged 5 commits into from
Jun 6, 2024

Conversation

derekblank
Copy link
Contributor

@derekblank derekblank commented Jun 5, 2024

Improves Assistant-related icons to match designs.

Proposed Changes

  • Removes static <ReturnIcon /> and <MenuIcon /> in favor of @wordpress/icons: keyboardReturn and moreVertical.
  • Updates <AssistantIcon /> to regular function component (#)

Testing Instructions

  1. Start app with AI Assistant flag enabled: STUDIO_AI=true npm start
  2. Navigate to Assistant tab
  3. Type text in the <AIInput /> component
  4. Observe the following icons (left to right -- Assistant, keyboardReturn, and moreVertical):
Icons

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@derekblank derekblank added the [Type] Enhancement Improvement upon an existing feature label Jun 5, 2024
@derekblank derekblank requested review from dcalhoun and a team June 5, 2024 11:22
@derekblank derekblank self-assigned this Jun 5, 2024
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

The new icons look good to me. 🚀

src/components/ai-input.tsx Outdated Show resolved Hide resolved
</div>
) }
<div className="flex items-end p-2">
<div className="flex items-end py-2">
Copy link
Member

Choose a reason for hiding this comment

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

The horizontal padding removal creates collision between the input and button focus styles, I feel space between the elements is beneficial. I wonder if we might retain the padding or address this in a different manner. WDYT?

Semi-related: I note the focus style color is different between these two elements.

Overlapping input and button focus styles

Copy link
Contributor Author

@derekblank derekblank Jun 6, 2024

Choose a reason for hiding this comment

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

Good catch. I transferred the button spacing to the margin to preserve the unfocused layout styles, and updated the AIInput focus color to match the Button focus color (and the rest of Studio's input components). 👍

aiinput-border.mov

@derekblank derekblank merged commit ae3de99 into trunk Jun 6, 2024
11 checks passed
@derekblank derekblank deleted the fix/update-icons branch June 6, 2024 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improvement upon an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants