feat: add tooltip support to MultiSelect widget#41726
feat: add tooltip support to MultiSelect widget#41726fffzzau wants to merge 3 commits intoappsmithorg:releasefrom
Conversation
WalkthroughAdded configurable tooltip support to the MultiSelect widget (prop added, component wrapped in Tooltip). Minor config updates: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/client/src/widgets/MultiSelectWidget/component/index.tsx (1)
187-187: Guard against whitespace-only tooltip content.
disabled={!tooltip}treats" "as truthy and still renders an effectively empty tooltip.Proposed fix
- <Tooltip content={tooltip} disabled={!tooltip} position={Position.TOP}> + <Tooltip + content={tooltip?.trim()} + disabled={!tooltip?.trim()} + position={Position.TOP} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/MultiSelectWidget/component/index.tsx` at line 187, The Tooltip is currently disabled using disabled={!tooltip}, which treats whitespace-only strings as truthy and still shows an empty tooltip; update the condition for Tooltip (the Tooltip usage in component/index.tsx) to consider trimmed content (e.g., treat tooltip as empty when tooltip.trim() is an empty string) so the disabled prop becomes true for null/undefined/empty/whitespace-only tooltip values; apply this check wherever Tooltip is rendered in this component to prevent rendering whitespace-only tooltips.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 47: The APPSMITH_CLOUD_SERVICES_BASE_URL entry in .env.example is wrapped
in quotes which is inconsistent with other variables; remove the surrounding
double quotes from the APPSMITH_CLOUD_SERVICES_BASE_URL value so it matches the
unquoted style used throughout the file, and confirm this unrelated change
(APPSMITH_CLOUD_SERVICES_BASE_URL) is intentional for this PR or move it to a
separate commit/PR if it does not belong to the MultiSelect tooltip work.
In `@app/client/src/widgets/MultiSelectWidget/component/index.tsx`:
- Around line 20-21: Replace the Blueprint tooltip usage by importing Tooltip
from "@design-system/ads" instead of "Tooltip2" from "@blueprintjs/popover2"
(update the import that currently reads "Tooltip2 as Tooltip"); also change any
prop usages position={Position.TOP} to placement="top" and disabled={!tooltip}
to isDisabled={!tooltip}; ensure you remove the unused Position import from
"@blueprintjs/core" and keep LabelWithTooltip usage consistent.
In `@app/client/tsconfig.json`:
- Line 4: Update the tsconfig setting "ignoreDeprecations" to match the repo's
TypeScript version: change the value of the "ignoreDeprecations" property from
"6.0" to "5.0" in app/client/tsconfig.json so it targets TypeScript 5.x
deprecation suppression.
---
Nitpick comments:
In `@app/client/src/widgets/MultiSelectWidget/component/index.tsx`:
- Line 187: The Tooltip is currently disabled using disabled={!tooltip}, which
treats whitespace-only strings as truthy and still shows an empty tooltip;
update the condition for Tooltip (the Tooltip usage in component/index.tsx) to
consider trimmed content (e.g., treat tooltip as empty when tooltip.trim() is an
empty string) so the disabled prop becomes true for
null/undefined/empty/whitespace-only tooltip values; apply this check wherever
Tooltip is rendered in this component to prevent rendering whitespace-only
tooltips.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e460e349-e317-44fa-9a33-084e41278c10
📒 Files selected for processing (4)
.env.exampleapp/client/src/widgets/MultiSelectWidget/component/index.tsxapp/client/src/widgets/MultiSelectWidget/widget/index.tsxapp/client/tsconfig.json
| # Configure cloud services | ||
| # APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com" | ||
|
|
||
| APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com" |
There was a problem hiding this comment.
Remove quotes around the URL value for consistency.
The value is wrapped in quotes, which is inconsistent with all other environment variables in this file (lines 4-54) that don't use quotes. In .env files, quotes are unnecessary for URLs without spaces or special characters.
Additionally, this change appears unrelated to the PR objective of implementing a tooltip for MultiSelect. Please verify this change belongs in this PR.
♻️ Proposed fix
-APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com"
+APPSMITH_CLOUD_SERVICES_BASE_URL=https://release-cs.appsmith.com📝 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.
| APPSMITH_CLOUD_SERVICES_BASE_URL="https://release-cs.appsmith.com" | |
| APPSMITH_CLOUD_SERVICES_BASE_URL=https://release-cs.appsmith.com |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 47-47: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example at line 47, The APPSMITH_CLOUD_SERVICES_BASE_URL entry in
.env.example is wrapped in quotes which is inconsistent with other variables;
remove the surrounding double quotes from the APPSMITH_CLOUD_SERVICES_BASE_URL
value so it matches the unquoted style used throughout the file, and confirm
this unrelated change (APPSMITH_CLOUD_SERVICES_BASE_URL) is intentional for this
PR or move it to a separate commit/PR if it does not belong to the MultiSelect
tooltip work.
| { | ||
| "extends": "./tsconfig.path.json", | ||
| "compilerOptions": { | ||
| "ignoreDeprecations": "6.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify TS version and ignoreDeprecations compatibility in this repo.
set -euo pipefail
echo "== Declared TypeScript versions =="
fd -HI '^package\.json$' | xargs -I{} rg -nP '"typescript"\s*:\s*"' "{}"
echo
echo "== ignoreDeprecations settings =="
rg -nP '"ignoreDeprecations"\s*:' app/client/tsconfig.jsonRepository: appsmithorg/appsmith
Length of output: 223
🏁 Script executed:
cat app/client/tsconfig.jsonRepository: appsmithorg/appsmith
Length of output: 1087
🏁 Script executed:
fd -HI 'tsconfig.*\.json$' --max-results 20Repository: appsmithorg/appsmith
Length of output: 810
🌐 Web query:
TypeScript ignoreDeprecations "6.0" version meaning 5.5 compatibility
💡 Result:
compilerOptions.ignoreDeprecations takes a TypeScript version string that acts like a “deprecation cutoff”: you’re telling tsc “don’t report deprecation diagnostics up through (and including) this version’s deprecations, so I can upgrade in steps.” In TypeScript 6.0 specifically, the TS team documents using "ignoreDeprecations": "6.0" to temporarily silence 6.0 deprecation warnings (e.g. around deprecated syntax), with the expectation you should fix them before moving on to future major versions. [1]
For TypeScript 5.5, the documented value used to keep older deprecated options working/silenced is "5.0" (not "6.0"). [2]
Also, the compiler’s option parsing (in the 5.x→6.0 transition) indicates that acceptance is explicitly keyed to specific strings (e.g. "5.0" or "6.0"), so "6.0" is not a generic “any future version” marker and may not be accepted/meaningful in older compiler versions. [3]
Practical takeaway
- If you’re on TS 5.5: use
"ignoreDeprecations": "5.0". [2] - Use
"ignoreDeprecations": "6.0"when you’re actually on TS 6.0 and want to temporarily silence 6.0 deprecations while you migrate. [1]
References:
[1] TypeScript 6.0 RC announcement (Microsoft DevBlogs) (devblogs.microsoft.com)
[2] TypeScript 5.5 RC announcement (Microsoft DevBlogs) (devblogs.microsoft.com)
[3] 5.x→6.0 migration guide noting option-value handling (gist.github.com)
Citations:
- 1: https://devblogs.microsoft.com/typescript/announcing-typescript-6-0-rc/?utm_source=openai
- 2: https://devblogs.microsoft.com/typescript/announcing-typescript-5-5-rc/?utm_source=openai
- 3: https://gist.github.com/privatenumber/3d2e80da28f84ee30b77d53e1693378f?permalink_comment_id=6001541&utm_source=openai
🏁 Script executed:
rg '"ignoreDeprecations"' app/client/ -A 1 -B 1Repository: appsmithorg/appsmith
Length of output: 215
Change ignoreDeprecations to "5.0" to match TypeScript 5.5.4.
The setting "ignoreDeprecations": "6.0" targets TypeScript 6.0 deprecations, but the repo uses TypeScript 5.5.4. Use "5.0" instead per TypeScript's documented versioning for deprecation suppression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/tsconfig.json` at line 4, Update the tsconfig setting
"ignoreDeprecations" to match the repo's TypeScript version: change the value of
the "ignoreDeprecations" property from "6.0" to "5.0" in
app/client/tsconfig.json so it targets TypeScript 5.x deprecation suppression.
fffzzau
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/client/src/widgets/MultiSelectWidget/component/index.tsx (1)
193-193: UseClasses.SKELETONinstead of hardcoded"bp3-skeleton"for consistency.Line 147 already uses
Classes.SKELETONwhile line 193 hardcodes the Blueprint versioned class. Use the constant to avoid version-prefix drift and remove the inline comment.Suggested patch
- <MultiSelectContainer - className={loading ? "bp3-skeleton" : ""} // Replacing Classes.SKELETON with string if needed + <MultiSelectContainer + className={loading ? Classes.SKELETON : ""} compactMode={compactMode} data-testid="multiselect-container" isValid={isValid}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/widgets/MultiSelectWidget/component/index.tsx` at line 193, Replace the hardcoded "bp3-skeleton" string in the JSX className expression with the Blueprint constant Classes.SKELETON for consistency: locate the className prop in the MultiSelectWidget component where it currently does className={loading ? "bp3-skeleton" : ""} and change it to use Classes.SKELETON (ensure Classes is imported from `@blueprintjs/core` if not already) and remove the inline comment so the expression becomes className={loading ? Classes.SKELETON : ""}.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/client/src/widgets/MultiSelectWidget/component/index.tsx`:
- Line 193: Replace the hardcoded "bp3-skeleton" string in the JSX className
expression with the Blueprint constant Classes.SKELETON for consistency: locate
the className prop in the MultiSelectWidget component where it currently does
className={loading ? "bp3-skeleton" : ""} and change it to use Classes.SKELETON
(ensure Classes is imported from `@blueprintjs/core` if not already) and remove
the inline comment so the expression becomes className={loading ?
Classes.SKELETON : ""}.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17396442-ead3-4c5b-91bd-5c301662acf1
📒 Files selected for processing (1)
app/client/src/widgets/MultiSelectWidget/component/index.tsx
1 participant
I have implemented a new tooltip property for the MultiSelect widget. This allows developers to provide extra context or instructions to end-users when they hover over the dropdown.
Changes made:
Added tooltip to the MultiSelect widget property pane.
Wrapped the MultiSelect component in the ADS (Appsmith Design System) Tooltip.
The tooltip only renders when it contains non-whitespace text.
Updated widget configuration to handle the new property.
Fixes: #41726
Summary by CodeRabbit
New Features
Chores