Skip to content

fix(core): ai apply ui opt #13238

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 1 commit into
base: canary
Choose a base branch
from
Open

fix(core): ai apply ui opt #13238

wants to merge 1 commit into from

Conversation

yoyoyohamapi
Copy link
Contributor

@yoyoyohamapi yoyoyohamapi commented Jul 16, 2025

CLOSE AI-377 AI-372 AI-373 AI-381 AI-378 AI-374 AI-382 AI-375

Summary by CodeRabbit

  • Style

    • Improved button styling for tool controls, including hover effects and consistent padding.
    • Updated background colors and border placements for result cards and headers.
  • New Features

    • Added tooltips to control buttons for enhanced user guidance.
  • Bug Fixes

    • Improved accessibility by replacing clickable spans with button elements.
    • Updated loading indicators to use a spinner icon for clearer feedback during actions.
  • Refactor

    • Simplified layout and reduced unnecessary wrapper elements for cleaner rendering.

@yoyoyohamapi yoyoyohamapi requested a review from akumatus July 16, 2025 07:37
@yoyoyohamapi yoyoyohamapi requested a review from a team as a code owner July 16, 2025 07:37
Copy link
Contributor

coderabbitai bot commented Jul 16, 2025

Walkthrough

The update refactors the UI and interaction elements in the doc-edit.ts file, replacing spans with buttons for accessibility, switching from an animated star icon to a loading spinner for loading states, refining borders and padding, and adding tooltips and hover effects for improved user experience.

Changes

File(s) Change Summary
packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts Replaced spans with buttons for controls, switched animated star icon to loading spinner, adjusted borders, padding, hover styles, and added tooltips.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant DocEditTool
    participant LoadingIcon

    User->>DocEditTool: Clicks "Apply" or "Accept"
    DocEditTool->>LoadingIcon: Show spinner
    DocEditTool-->>User: Display "Applying"/"Accepting..." with spinner
    DocEditTool->>DocEditTool: Process action
    DocEditTool-->>User: Update UI based on result
Loading

Possibly related PRs

Suggested labels

app:core

Suggested reviewers

  • pengx17

Poem

Buttons now shine where spans once lay,
Loading spinners twirl in a gentle ballet.
Tooltips whisper secrets, hover glows anew,
Borders and padding, all freshened for you.
The doc-edit tool hops with delight—
A rabbit's touch makes the UI just right! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3949714 and b9aa15b.

📒 Files selected for processing (1)
  • packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts (4 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts (1)
Learnt from: yifeiyin
PR: toeverything/AFFiNE#13056
File: blocksuite/affine/blocks/embed-doc/src/embed-linked-doc-block/styles.ts:12-14
Timestamp: 2025-07-06T22:07:55.476Z
Learning: The design token `layer/background/tertiary` exists and is widely used throughout the AFFiNE codebase with `unsafeCSSVarV2`. It's used for borders in embed blocks, backgrounds in UI components, and is part of the established theme system. Don't flag this token as missing.
🧬 Code Graph Analysis (1)
packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts (2)
blocksuite/affine/components/src/icons/text.ts (3)
  • ExpandFullIcon (414-417)
  • ExpandCloseIcon (419-422)
  • CopyIcon (139-142)
blocksuite/affine/components/src/icons/loading.ts (1)
  • LoadingIcon (4-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (49)
  • GitHub Check: fuzzing
  • GitHub Check: y-octo binding test on aarch64-pc-windows-msvc
  • GitHub Check: y-octo binding test on x86_64-apple-darwin
  • GitHub Check: y-octo binding test on x86_64-pc-windows-msvc
  • GitHub Check: loom thread test
  • GitHub Check: Run native tests
  • GitHub Check: Build AFFiNE native (aarch64-apple-darwin)
  • GitHub Check: Build AFFiNE native (x86_64-apple-darwin)
  • GitHub Check: E2E Mobile Test (3)
  • GitHub Check: E2E Mobile Test (4)
  • GitHub Check: Build AFFiNE native (aarch64-pc-windows-msvc)
  • GitHub Check: E2E Mobile Test (5)
  • GitHub Check: E2E BlockSuite Test (7)
  • GitHub Check: Build AFFiNE native (x86_64-pc-windows-msvc)
  • GitHub Check: E2E BlockSuite Test (2)
  • GitHub Check: E2E Test (1)
  • GitHub Check: E2E BlockSuite Test (9)
  • GitHub Check: E2E BlockSuite Test (8)
  • GitHub Check: E2E BlockSuite Test (10)
  • GitHub Check: E2E Mobile Test (2)
  • GitHub Check: E2E Mobile Test (1)
  • GitHub Check: E2E BlockSuite Test (6)
  • GitHub Check: E2E Test (5)
  • GitHub Check: E2E BlockSuite Test (4)
  • GitHub Check: E2E Test (9)
  • GitHub Check: E2E BlockSuite Test (5)
  • GitHub Check: E2E BlockSuite Test (3)
  • GitHub Check: E2E Test (8)
  • GitHub Check: E2E BlockSuite Test (1)
  • GitHub Check: E2E BlockSuite Cross Browser Test (1, chromium)
  • GitHub Check: E2E Test (10)
  • GitHub Check: E2E BlockSuite Cross Browser Test (2, webkit)
  • GitHub Check: E2E Test (4)
  • GitHub Check: E2E Test (7)
  • GitHub Check: E2E Test (2)
  • GitHub Check: E2E BlockSuite Cross Browser Test (1, firefox)
  • GitHub Check: E2E BlockSuite Cross Browser Test (2, chromium)
  • GitHub Check: E2E BlockSuite Cross Browser Test (1, webkit)
  • GitHub Check: E2E Test (3)
  • GitHub Check: E2E Test (6)
  • GitHub Check: E2E BlockSuite Cross Browser Test (2, firefox)
  • GitHub Check: Typecheck
  • GitHub Check: Build Server native
  • GitHub Check: Analyze (typescript, affine)
  • GitHub Check: Analyze (typescript, blocksuite)
  • GitHub Check: Analyze (javascript, blocksuite)
  • GitHub Check: Build @affine/electron renderer
  • GitHub Check: Analyze (javascript, affine)
  • GitHub Check: Lint
🔇 Additional comments (8)
packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts (8)

5-5: Good improvement: Loading icon import

The change from AIStarIconWithAnimation to LoadingIcon provides better visual consistency for loading states throughout the application.


138-138: Good styling improvement: Content border

Adding a top border to separate the content section improves visual hierarchy and structure.


120-131: Excellent accessibility improvement: Button styling

Converting clickable spans to proper buttons with hover states significantly improves accessibility and user experience. The styling is consistent and well-implemented.


149-159: Good consistency: Footer button styling

The footer button styling matches the header operations and maintains visual consistency throughout the component.


435-444: Excellent UX improvements: Tooltips and proper buttons

The conversion of spans to buttons with tooltips for collapse/expand and copy operations greatly improves accessibility and user experience. The tooltip text is clear and helpful.


450-451: Good loading state improvement: Apply button

Replacing the animated star with LoadingIcon and "Applying" text provides clearer feedback about the loading state.


456-482: Excellent accessibility refactor: Footer controls

Converting the footer controls from divs to proper buttons with appropriate styling, disabled states, and visual feedback significantly improves accessibility and user experience. The loading states are well-implemented.


475-481: Good loading state consistency: Accept button

Using LoadingIcon for the accept button loading state maintains consistency with the apply button and provides clear visual feedback. The disabled styling with reduced opacity is appropriate.

@@ -83,7 +83,7 @@ export class DocEditTool extends WithDisposable(ShadowlessElement) {
display: flex;
flex-direction: column;
align-items: flex-start;
background: ${unsafeCSSVarV2('layer/background/overlayPanel')};
background: ${unsafeCSSVar('--affine-overlay-panel-shadow')};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Potential issue: Inconsistent CSS variable usage

The background color change from unsafeCSSVarV2('layer/background/overlayPanel') to unsafeCSSVar('--affine-overlay-panel-shadow') appears problematic:

  1. Mixed usage of unsafeCSSVar and unsafeCSSVarV2 functions
  2. The new variable name suggests it's for shadow, not background
  3. This may not provide the intended visual styling

Consider reverting to the original variable or using the correct background variable:

-      background: ${unsafeCSSVar('--affine-overlay-panel-shadow')};
+      background: ${unsafeCSSVarV2('layer/background/overlayPanel')};

Or if a different background is needed, use the appropriate variable with consistent function:

-      background: ${unsafeCSSVar('--affine-overlay-panel-shadow')};
+      background: ${unsafeCSSVarV2('layer/background/tertiary')};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
background: ${unsafeCSSVar('--affine-overlay-panel-shadow')};
// revert to the original overlaypanel background
background: ${unsafeCSSVarV2('layer/background/overlayPanel')};
Suggested change
background: ${unsafeCSSVar('--affine-overlay-panel-shadow')};
// or, if you want a tertiary background instead
background: ${unsafeCSSVarV2('layer/background/tertiary')};
🤖 Prompt for AI Agents
In packages/frontend/core/src/blocksuite/ai/components/ai-tools/doc-edit.ts at
line 86, the background CSS variable was changed from using unsafeCSSVarV2 with
'layer/background/overlayPanel' to unsafeCSSVar with
'--affine-overlay-panel-shadow', which is inconsistent and likely incorrect for
background styling. To fix this, revert to using the original
unsafeCSSVarV2('layer/background/overlayPanel') for the background property or
replace it with the correct background-related CSS variable using the same
unsafeCSSVarV2 function to maintain consistency and ensure the intended visual
style.

Copy link

codecov bot commented Jul 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.86%. Comparing base (04e002e) to head (b9aa15b).
Report is 7 commits behind head on canary.

Additional details and impacted files
@@            Coverage Diff             @@
##           canary   #13238      +/-   ##
==========================================
- Coverage   56.87%   56.86%   -0.02%     
==========================================
  Files        2709     2711       +2     
  Lines      132995   133006      +11     
  Branches    20590    20591       +1     
==========================================
- Hits        75647    75630      -17     
- Misses      55678    55705      +27     
- Partials     1670     1671       +1     
Flag Coverage Δ
server-test 78.65% <ø> (-0.03%) ⬇️
unittest 32.22% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@akumatus akumatus enabled auto-merge July 16, 2025 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants