Skip to content

Conversation

@chenxi-20
Copy link
Collaborator

@chenxi-20 chenxi-20 commented Sep 16, 2025

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Style

    • Adjusted tab title in non-stretch headers to block-level with full width, improving alignment and overflow handling.
    • Minor cleanup in header-only styles (whitespace/brace adjustments).
  • Refactor

    • Deferred DOM lookups for tooltip on tab title hover to run only when needed; no behavior or API changes.

@chenxi-20 chenxi-20 added the bug Something isn't working label Sep 16, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 16, 2025

Walkthrough

Reorders variable declarations in a tab title mouseenter handler to occur after a tooltip-config guard. Updates tabs CSS: makes non-stretch header tab item titles block-level with width 100%, and performs minor formatting/brace adjustments. No public API changes.

Changes

Cohort / File(s) Summary
Tab nav mouseenter handler
packages/renderless/src/tab-nav/index.ts
Moved dom/el declarations below the tooltipConfig early-return guard; overflow check and tooltip setup logic unchanged; no API changes.
Tabs theme styles
packages/theme/src/tabs/index.less
In non-stretch header nav, .tab-item .title changed from display: inline-block to display: block; width: 100%; minor whitespace/brace tidy-ups; no new CSS rules added.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant T as Tab Title
  participant H as Mouseenter Handler
  participant TT as Tooltip

  U->>T: hover
  T->>H: handleTitleMouseenter(event)
  alt tooltipConfig disabled/guarded
    H-->>T: return early (no work)
  else tooltipConfig enabled
    H->>H: resolve event target (dom) and title el (el)
    H->>H: check overflow (el.scrollWidth > el.offsetWidth)
    alt overflow
      H->>TT: configure and show tooltip
    else no overflow
      H-->>TT: ensure tooltip hidden
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my ears at tidy flow,
A hover hush, then tooltips show.
Tabs now stretch with blocky grace,
Titles fill their cozy space.
Two files nibbled, neat and slight—
Carrots cached, the code feels right. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change — a bugfix for using overflow-title together with with-close on tabs — and aligns with the code and CSS edits in the PR that address tooltip/overflow and title layout; it is specific and clear enough for a reviewer to understand the main intent without extraneous detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/tabs-20250916

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@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: 0

🧹 Nitpick comments (2)
packages/renderless/src/tab-nav/index.ts (1)

437-452: Anchor tooltip to currentTarget; avoid bubbling to child nodes (e.g., close icon)

Using e.currentTarget is more robust than e.target for mouseenter. Also cast to HTMLElement for clarity.

Apply:

-    const dom = e.target
-    // 以下逻辑与aui不同,tiny不能通过dom宽度判断是否超出隐藏
-    const el = title?.el
+    const dom = (e.currentTarget || e.target) as HTMLElement
+    // 以下逻辑与aui不同,tiny不能通过dom宽度判断是否超出隐藏
+    const el = title?.el as HTMLElement | undefined
packages/theme/src/tabs/index.less (1)

118-124: Use flex growth + min-width: 0 instead of width: 100% in flex row

width: 100% on a flex item can push/overlap the close icon. Let the title grow to available space and truncate correctly.

Apply:

-        .@{tabs-prefix-cls}__item__title {
-          display: block;
-          width: 100%;
+        .@{tabs-prefix-cls}__item__title,
+        .@{tabs-prefix-cls}__item-title {
+          display: block;
+          flex: 1 1 auto;
+          min-width: 0;
           white-space: nowrap;
           overflow: hidden;
           text-overflow: ellipsis;
         }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7284629 and 1ca6587.

📒 Files selected for processing (2)
  • packages/renderless/src/tab-nav/index.ts (1 hunks)
  • packages/theme/src/tabs/index.less (1 hunks)
⏰ 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). (1)
  • GitHub Check: PR E2E Test (pnpm test:e2e3)
🔇 Additional comments (2)
packages/renderless/src/tab-nav/index.ts (1)

437-440: Good: defer DOM reads until after tooltipConfig guard

This avoids unnecessary DOM access and minor work on early-return paths.

packages/theme/src/tabs/index.less (1)

118-124: Confirm BEM selector actually matches DOM

Both .@{tabs-prefix-cls}__item__title (double-underscore) and .@{tabs-prefix-cls}__item-title (single-hyphen, from the nested &__item &-title) are present in packages/theme/src/tabs/index.less; repository search returned no template/markup usages (rg skipped files). Verify which class the markup uses and remove the unused selector to avoid dead CSS.

@zzcr zzcr merged commit 33358da into dev Sep 18, 2025
10 of 11 checks passed
@zzcr zzcr deleted the fix/tabs-20250916 branch September 18, 2025 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants