feat: added Tooltip property for Checkbox widget#41483
Conversation
amelia-c0n
requested review from
vivek-appsmith
and removed request for
a team
WalkthroughAdds tooltip support to the checkbox widget via a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/client/src/widgets/CheckboxWidget/component/index.tsx (1)
62-97: Remove unusedCheckboxLabelcomponent.The
CheckboxLabelstyled component is no longer used after the replacement withLabelWithTooltip. Dead code should be removed to improve maintainability.🔎 Proposed fix
-export const CheckboxLabel = styled.div<{ - disabled?: boolean; - alignment: AlignWidgetTypes; - labelTextColor?: string; - labelTextSize?: string; - labelStyle?: string; - isDynamicHeightEnabled?: boolean; - isLabelInline?: boolean; -}>` - width: 100%; - display: inline-block; - vertical-align: top; - text-align: ${({ alignment }) => alignment.toLowerCase()}; - ${({ disabled, labelStyle, labelTextColor, labelTextSize }) => ` - color: ${ - disabled ? "var(--wds-color-text-disabled)" : labelTextColor || "inherit" - }; - font-size: ${labelTextSize ?? "inherit"}; - font-weight: ${labelStyle?.includes(FontStyleTypes.BOLD) ? "bold" : "normal"}; - font-style: ${ - labelStyle?.includes(FontStyleTypes.ITALIC) ? "italic" : "normal" - }; - `} - - ${({ isDynamicHeightEnabled }) => - isDynamicHeightEnabled ? "&& { word-break: break-all; }" : ""}; - - ${({ isLabelInline }) => - isLabelInline && - ` - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - word-wrap: normal; - `} -`; -
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoreapp/client/src/widgets/CheckboxWidget/component/index.tsxapp/client/src/widgets/CheckboxWidget/widget/index.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-21T07:25:06.064Z
Learnt from: CR
Repo: appsmithorg/appsmith PR: 0
File: .cursor/rules/README.md:0-0
Timestamp: 2025-07-21T07:25:06.064Z
Learning: Applies to **/*.{mdc} : Each rule must be defined in a Markdown Cursor (.mdc) file with metadata, logic (JavaScript), and documentation
Applied to files:
.gitignore
📚 Learning: 2025-04-29T09:12:14.552Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 40462
File: app/client/src/instrumentation/index.ts:0-0
Timestamp: 2025-04-29T09:12:14.552Z
Learning: Only comment on files that are directly related to the PR's objectives, even if other files appear in the diff. For PR #40462, the focus is on the import override feature for artifacts, not on instrumentation or telemetry files.
Applied to files:
.gitignore
📚 Learning: 2024-11-13T09:13:11.481Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/src/widgets/ImageWidget/widget/index.tsx:382-382
Timestamp: 2024-11-13T09:13:11.481Z
Learning: In the `ImageWidget` component (`app/client/src/widgets/ImageWidget/widget/index.tsx`), the `alt` property should remain optional (`alt?: string;`) because there are fallback values when the user does not provide alternative text.
Applied to files:
app/client/src/widgets/CheckboxWidget/widget/index.tsx
🧬 Code graph analysis (1)
app/client/src/widgets/CheckboxWidget/component/index.tsx (1)
app/client/packages/design-system/widgets/src/components/ToolbarButtons/stories/ToolbarButtons.stories.tsx (1)
Alignment(89-96)
⏰ 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). (5)
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (9)
.gitignore (1)
1-2: Clarify the necessity and scope of .gitignore changes.The two new patterns (
specs/quick-fixes/**and.cursor/rules/quick-fix.mdc) appear to be infrastructure/tooling-related changes rather than directly supporting the checkbox tooltip feature. Per learnings, focus should be on files directly related to PR objectives.Consider whether these changes should be included in this PR or separated into a dedicated infrastructure update.
app/client/src/widgets/CheckboxWidget/component/index.tsx (5)
5-5: LGTM! Imports are correctly added for tooltip support.The new imports (
Alignment,Classes,LabelWithTooltip,LABEL_CONTAINER_CLASS) are properly utilized throughout the component to enable tooltip functionality.Also applies to: 10-12
20-20: LGTM! Container props extended correctly.The
alignWidgetprop is properly added to enable alignment-based styling for the label container.
41-58: LGTM! Alignment-aware styling is well-implemented.The label container styling correctly handles both left and right alignment scenarios with appropriate flex and text properties. The text wrapping and overflow handling is appropriate.
130-130: LGTM! Alignment prop correctly propagated.The
alignWidgetprop is properly passed toCheckboxContainerto enable alignment-based styling.
201-201: LGTM! Props interface correctly extended.The
labelTooltipproperty is properly added as an optional string, consistent with other label-related props.app/client/src/widgets/CheckboxWidget/widget/index.tsx (3)
203-212: LGTM! Property pane configuration is well-structured.The
labelTooltipproperty is correctly configured with appropriate control type, help text, and validation. The placement in the "General" section is logical.
481-481: LGTM! Prop correctly wired to component.The
labelTooltipprop is properly passed from the widget to the component, following the established pattern.
518-518: LGTM! Widget props interface correctly extended.The
labelTooltipproperty is properly added as an optional string, consistent with the component props and other similar properties.
| <LabelWithTooltip | ||
| alignment={ | ||
| this.props.alignWidget === AlignWidgetTypes.RIGHT | ||
| ? Alignment.RIGHT | ||
| : Alignment.LEFT | ||
| } | ||
| className="t--checkbox-widget-label" | ||
| color={this.props.labelTextColor} | ||
| compact | ||
| disabled={this.props.isDisabled} | ||
| fontSize={this.props.labelTextSize} | ||
| fontStyle={this.props.labelStyle} | ||
| helpText={this.props.labelTooltip} | ||
| inline={this.props.isLabelInline} | ||
| isDynamicHeightEnabled={this.props.isDynamicHeightEnabled} | ||
| isLabelInline={this.props.isLabelInline} | ||
| labelStyle={this.props.labelStyle} | ||
| labelTextColor={this.props.labelTextColor} | ||
| labelTextSize={this.props.labelTextSize} | ||
| > | ||
| {this.props.label} | ||
| </CheckboxLabel> | ||
| loading={this.props.isLoading} | ||
| optionCount={1} | ||
| position={this.props.labelPosition} | ||
| text={this.props.label} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f \( -name "*.tsx" -o -name "*.ts" \) | xargs grep -l "LabelWithTooltip" | head -10Repository: appsmithorg/appsmith
Length of output: 699
🏁 Script executed:
cat ./app/client/packages/design-system/widgets-old/src/LabelWithTooltip/index.tsxRepository: appsmithorg/appsmith
Length of output: 8275
Remove unsupported isDynamicHeightEnabled prop from LabelWithTooltip.
The component doesn't support the isDynamicHeightEnabled prop—it's not defined in the LabelWithTooltipProps interface. While all other props are correctly supported, this unknown prop should be removed to avoid silent failures and maintain type safety.
🤖 Prompt for AI Agents
In app/client/src/widgets/CheckboxWidget/component/index.tsx around lines 147 to
166 the JSX passes an unsupported prop isDynamicHeightEnabled to
LabelWithTooltip which is not defined on LabelWithTooltipProps; remove the
isDynamicHeightEnabled={this.props.isDynamicHeightEnabled} prop from the
LabelWithTooltip element so only supported props are passed (if dynamic-height
behavior is required, map it to a supported prop or implement the feature on
LabelWithTooltip and add the prop to its props interface).
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20467649657. |
|
Deploy-Preview-URL: https://ce-41483.dp.appsmith.com |
| .${LABEL_CONTAINER_CLASS} { | ||
| margin-right: 0px; | ||
| max-width: 100%; | ||
| width: 100%; | ||
| flex: 1 1 auto; | ||
| flex-wrap: wrap; | ||
| justify-content: ${({ alignWidget }) => | ||
| alignWidget === AlignWidgetTypes.RIGHT ? "flex-end" : "flex-start"}; | ||
|
|
||
| label { | ||
| text-align: ${({ alignWidget }) => | ||
| alignWidget === AlignWidgetTypes.RIGHT ? "right" : "left"}; | ||
| white-space: normal; | ||
| overflow: auto; | ||
| text-overflow: unset; | ||
| word-break: break-word; | ||
| } | ||
| } |
There was a problem hiding this comment.
Why are we adding more styles to label container? I believe we pass alignment prop to the so as to adjust according to alignment settings
Also the style properties seems a bit off. Why do we need max-width width and flex in the same component with all of them maxed out to full width?
There was a problem hiding this comment.
The reasons I added the additional styles for the label container are:
LabelWithTooltipdoesn’t apply alignment for the Position = Right case. The label stays left-aligned for the scenario Position = Right + Alignment = Right. To fix this, I introduced thejustify-contentandlabel text-align.- Overrides such as
max-widthandflexare layout tweaks to ensure the label aligns to the edge of the widget with no offsets (e.g., if Position = Left + Alignment = Left). However, I can remove thewidthandmargin-right, since they are redundant. - Overrides such as
overflowensure the behavior matches the previous behavior of the Checkbox widget for cases such as Fixed Height + long labels (the label wraps, no ellipsis).
If we skip the LABEL_CONTAINER_CLASS overrides altogether, this is the behavior we get: screen recording.
Let me know your thoughts.
| hasError={!isValid} | ||
| inputRef={this.props.inputRef} | ||
| labelElement={ | ||
| <CheckboxLabel |
There was a problem hiding this comment.
Since CheckboxLabel is not used, can we remove the code for it?
There was a problem hiding this comment.
The CheckboxLabel is still used by the CheckboxGroupWidget for the labels of the individual options and for the Select All option, so removing it would break that dependency.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Input/Input_spec.js (2)
164-164: Pre-existing: Avoidcy.wait(1000).Per coding guidelines,
cy.waitwith arbitrary timeouts should be avoided. Consider replacing with a more deterministic wait (e.g., waiting for a specific element state or network request).
174-176: Pre-existing: RemoveafterEachblock.Per coding guidelines,
afterandafterEachhooks should be avoided in test cases. This emptyafterEachblock should be removed.Proposed fix
-afterEach(() => { - // put your clean up code if any -});
3 participants
Description
Added a Tooltip option for the Checkbox widget that enables developers to add hints or provide additional information to guide the user regarding the required input.
Fixes #41468
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/21445878024
Commit: 2bdb890
Cypress dashboard.
Tags:
@tag.AllSpec:
Wed, 28 Jan 2026 17:03:05 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.