Skip to content

Keep escaped anchor selectors updating - #1286

Merged
danielbarion merged 2 commits into
ReactTooltip:masterfrom
feritcemkahraman:fix/escaped-anchor-selectors
Oct 15, 2026
Merged

danielbarion merged 2 commits into
ReactTooltip:masterfrom
feritcemkahraman:fix/escaped-anchor-selectors

Conversation

@feritcemkahraman

@feritcemkahraman feritcemkahraman commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

With five or more tooltip selectors, anchors matched by CSS escapes such as [data-tooltip-id='\61 nchor'] stop updating when elements are inserted or their IDs change. The registry compares the escaped text with the unescaped dataset value.

Reuse the selector parser and route escapes it cannot decode through the existing native selector fallback. Plain IDs and escaped quotes keep their fast path.

Three regressions fail before this change and pass afterward. TypeScript and changed-file ESLint/Prettier pass. The full Jest run passed 179 tests and 8 snapshots but had three imperative/delay failures; the numeric-zero imperative failure also reproduces on unchanged HEAD, while the two delay failures vary between runs. All bundle builds and declarations pass. Browser matrix not run.

Summary by CodeRabbit

  • Bug Fixes
    • Improved tooltip anchor detection for escaped data-tooltip-id selectors. Hexadecimal and backslash escapes now remain consistent with native selector matching, while escaped apostrophes continue to be handled correctly. This helps ensure tooltip anchors are detected and updates are reported accurately when matching anchors are added or removed.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 95ab706f-6f93-4483-b553-4dc75d192807
📥 Commits

Reviewing files that changed from the base of the PR and between 03b0ef3 and 3dba977.

📒 Files selected for processing: 1
  • src/utils/parse-data-tooltip-id-selector.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The tooltip anchor registry now uses parseDataTooltipIdSelector to derive tooltip IDs. The parser rejects captured selector values with unsupported backslash escapes. Tests cover escape parsing, native selector matching, and subscriber updates.

Changes: Escaped selector handling

Layer / File(s) Summary
Parser and registry integration
src/utils/parse-data-tooltip-id-selector.ts, src/components/Tooltip/anchor-registry.ts, src/test/escaped-anchor-selectors.spec.js
The parser rejects captured values with a backslash not followed by a quote. The registry uses the shared parser instead of its local helper. Tests cover escape parsing, native selector matching, and subscription notifications.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: danielbarion

Merge Risk: ⚪ Minimal · up to 3dba9

Escaped anchor selectors are refreshed through native selector matching when relevant DOM mutations occur. The reviewed changes show no actionable merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving updates for anchors that use escaped selectors.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.

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.

@danielbarion danielbarion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix, I didn't realized that issue before!

Can you please update the regex code by the comment I did?

const match = selector.match(/^\[data-tooltip-id=(['"])((?:\\.|(?!\1).)*)\1\]$/)

if (!match) {
if (!match || /\\(?!['"])/.test(match[2])) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please improve that hard-coded regex in the conditional by adding it into a constant before it? Also can you please add two straightforward comments before each variable explaining it?

@danielbarion danielbarion self-assigned this Oct 14, 2026

@danielbarion danielbarion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@danielbarion
danielbarion merged commit 04bcacd into ReactTooltip:master Oct 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants