Thulasizwe/en/dropdown tag#3395
Conversation
WalkthroughThe updates introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dropdown
participant ReflistTag
participant Tooltip
User->>Dropdown: Selects value(s) (tags mode)
Dropdown->>ReflistTag: Render tag with props (label, value, icon, tooltip, etc.)
ReflistTag->>Tooltip: Wrap Tag with tooltip text
Tooltip-->>User: Displays styled tag with tooltip on hover
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
shesha-reactjs/src/components/refListDropDown/models.ts (1)
58-67:Details
❓ Verification inconclusive
Add brief jsdoc & run code-wide reference check for new
descriptionfieldThe optional
description?: stringaddition is sensible, however:
- There is no comment explaining the intent / UI usage – please add a short JSDoc so future devs know this is consumed as a tooltip.
- Ensure every place that constructs an
ISelectOptionnow initialises (or at least ignores)description, otherwiseundefinedtooltips may surface./** Optional text shown as tooltip when the option is rendered as a tag */ + description?: string;Consider running a quick repo-wide grep to verify call-sites:
🏁 Script executed:
rg "description:" --type ts,tsxLength of output: 62
🏁 Script executed:
#!/bin/bash # Locate all usages of ISelectOption to identify where options are constructed rg -n "ISelectOption" -g "*.ts" -g "*.tsx" # Search for any explicit description property assignments in TS/TSX files rg -n "description\s*:" -g "*.ts" -g "*.tsx"Length of output: 387
🏁 Script executed:
#!/bin/bash set -e echo "=== Listing some TS/TSX files ===" find . -maxdepth 5 -type f \( -iname "*.ts" -o -iname "*.tsx" \) | head -n 20 echo echo "=== Searching for ISelectOption usages ===" rg -n "ISelectOption" echo echo "=== Searching for any 'description' property assignments ===" rg -n "description\s*?:"Length of output: 292
🏁 Script executed:
#!/bin/bash # List top-level files and directories to confirm repository structure echo "=== Root directory listing ===" find . -maxdepth 2 | sed 's/^/ /' | head -n 50 # Locate any TS/TSX files in the repo (ignoring node_modules) echo echo "=== TS/TSX files found ===" find . -type f -not -path "*/node_modules/*" \( -iname "*.ts" -o -iname "*.tsx" \) | head -n 20 # Search for ISelectOption usages (include hidden/ignored to be safe) echo echo "=== Usages of ISelectOption ===" rg -n "ISelectOption" -uu # Search for any explicit description property assignments echo echo "=== 'description' property assignments ===" rg -n "description\s*?:" -uuLength of output: 738
TS/TSX files not detected–please verify manually
I wasn’t able to locate any
.ts/.tsxfiles (includingshesha-reactjs/src/components/refListDropDown/models.ts) in the sandbox. Please ensure you’re on the correct branch and that the files are checked out locally, then:
- Confirm that
models.tsexists and contains the newdescription?: stringfield.- Add a JSDoc comment above
descriptionto explain its UI usage, for example:/** Optional text shown as tooltip when the option is rendered as a tag */ description?: string;- Once the files are present, run a repo-wide search for
description:in TS/TSX files to ensure everyISelectOptioninstantiation handles it:rg "description:" --type ts,tsx- Verify that call-sites either initialize
descriptionor deliberately omit it to avoidundefinedtooltips.shesha-reactjs/src/designer-components/dropdown/utils.ts (1)
35-38: Default tag background changed to empty string – verify visual fallback
background.colorwas#fff; now it’s''.
Ant Design falls back totransparent, not theme‐based colour, so tags may become invisible on light backgrounds.Please confirm this is the intended design and that designer previews / runtime render as expected for all themes.
shesha-reactjs/src/components/refListDropDown/reflistTag.tsx (1)
13-19: Move cursor style from Tooltip to Tag for better UX.The cursor style should be applied to the interactive element (Tag) rather than the Tooltip wrapper.
return ( - <Tooltip title={tooltip} placement={placement as TooltipProps['placement']} style={{ cursor: 'pointer' }}> + <Tooltip title={tooltip} placement={placement as TooltipProps['placement']}> <Tag key={value} color={memoizedColor} icon={(icon && showIcon) && <Icon type={icon} />} - style={getTagStyle(tagStyle, !!color)} + style={{ ...getTagStyle(tagStyle, !!color), cursor: 'pointer' }} >{showItemName && label}</Tag> </Tooltip> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
shesha-reactjs/src/components/dropdown/dropdown.tsx(6 hunks)shesha-reactjs/src/components/dropdown/style.ts(2 hunks)shesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsx(4 hunks)shesha-reactjs/src/components/refListDropDown/genericRefListDropDown.tsx(5 hunks)shesha-reactjs/src/components/refListDropDown/models.ts(1 hunks)shesha-reactjs/src/components/refListDropDown/reflistTag.tsx(1 hunks)shesha-reactjs/src/designer-components/dropdown/settingsForm.ts(6 hunks)shesha-reactjs/src/designer-components/dropdown/utils.ts(1 hunks)shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts(1 hunks)shesha-reactjs/src/designer-components/refListStatus/index.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-attempt
🔇 Additional comments (14)
shesha-reactjs/src/designer-components/refListStatus/index.tsx (1)
51-52: Prop-drilling raw style object – double-check precedence
style={model.allStyles?.fullStyle ?? {}}forwards the entire computed style object directly toRefListStatus.
IfRefListStatusalready merges its own defaults with incomingstyle, this is fine; otherwise you may unintentionally override internal sizing/spacing.Please confirm
RefListStatushandles this gracefully (e.g., viastyle={…mergeStyles(defaults, props.style)}) to avoid regressions.shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts (1)
353-359:Details
❓ Verification inconclusive
Renamed
tooltip→customTooltip– ensure renderer supports the new propOnly this input uses
customTooltip; the rest of the settings form (e.g. line 409) still usestooltip.
If the form engine has not been updated to recognisecustomTooltip, the help text will disappear.Please verify the underlying component API or align naming for consistency.
🏁 Script executed:
#!/usr/bin/env bash set -e echo "=== Searching for customTooltip usage in settingsForm.ts ===" rg "customTooltip" -n shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts || echo "No occurrences of customTooltip in settingsForm.ts" echo echo "=== Searching for tooltip usage in settingsForm.ts ===" rg "tooltip" -n shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts || echo "No occurrences of tooltip in settingsForm.ts" echo echo "=== Searching for customTooltip handling across codebase ===" rg "customTooltip" -n shesha-reactjs/src || echo "No customTooltip handlers found" echo echo "=== Searching for tooltip handling across codebase ===" rg "tooltip" -n shesha-reactjs/src || echo "No tooltip handlers found"Length of output: 1460
🏁 Script executed:
#!/usr/bin/env bash set -e echo "=== Locating settingsForm.ts files ===" FILES=$(find . -type f -name "settingsForm.ts") if [ -z "$FILES" ]; then echo "No settingsForm.ts found" exit 0 fi for f in $FILES; do echo echo "=== Inspecting $f ===" echo ">> customTooltip occurrences:" rg -n "customTooltip" "$f" || echo " (none)" echo echo ">> tooltip occurrences:" rg -n "tooltip" "$f" || echo " (none)" doneLength of output: 228
🏁 Script executed:
#!/usr/bin/env bash set -e echo "=== Searching for 'customDropdownMode' occurrences ===" rg -n "customDropdownMode" . echo echo "=== Searching for 'Dialog Width' occurrences ===" rg -n "Dialog Width" . echo echo "=== Searching for 'entityPicker' directory or usage ===" rg -n "entityPicker" .Length of output: 162
🏁 Script executed:
#!/usr/bin/env bash set -e echo "=== Listing top-level directories and subdirectories ===" find . -maxdepth 2 -type dLength of output: 220
Verify tooltip prop renaming support
It looks like the “Dialog Width” setting in your
settingsForm.tsnow usescustomTooltipinstead of the existingtooltipprop. If the form renderer or underlying component API hasn’t been updated to handlecustomTooltip, your help text will stop showing up.Please manually confirm and align naming:
• File (approx.):
shesha-reactjs/src/designer-components/entityPicker/settingsForm.ts
– Line withcustomTooltip: 'You can use any unit…'
• Check that the form engine/component acceptscustomTooltip(or update it).
• Either rename all remainingtooltipprops tocustomTooltip, or add support for both props for consistency.shesha-reactjs/src/components/refListDropDown/genericRefListDropDown.tsx (2)
1-8: LGTM! Import changes align with ReflistTag introduction.The removal of unused
Tagimport and addition ofReflistTagimport properly reflects the component refactoring.
120-133: LGTM! Properly forwards new display props to read-only component.The addition of
showItemNameprop ensures consistent behavior between editable and read-only modes.shesha-reactjs/src/components/readOnlyDisplayFormItem/index.tsx (3)
1-10: LGTM! Import changes properly reflect component usage.Removal of unused
Tagimport and addition ofReflistTagaligns with the refactoring.
81-92: LGTM! Correct implementation of ReflistTag for single dropdown.The component properly uses
descriptionfor tooltip and passes all required props. The use of nullish coalescing operator (??) is also a good improvement.
105-117: LGTM! Consistent ReflistTag implementation for multiple dropdowns.The implementation maintains consistency with the single dropdown version and properly handles array mapping.
shesha-reactjs/src/components/dropdown/dropdown.tsx (2)
11-11: LGTM! Import and style hook updates align with refactoring.The addition of ReflistTag import and simplified useStyles call properly reflect the component changes.
Also applies to: 41-41
83-84: LGTM! Addition of description property enhances tooltip support.Including the description in labeled values enables proper tooltip display in ReflistTag.
Also applies to: 109-110
shesha-reactjs/src/designer-components/dropdown/settingsForm.ts (5)
83-105: LGTM! Well-structured common settings reorganization.The reorganization into input rows improves the form layout and user experience.
107-139: LGTM! Proper conditional visibility for tag-specific controls.The conditional display of showItemName and showIcon based on displayStyle is well-implemented with appropriate tooltips.
451-467: LGTM! Clear display style options.The dropdown provides clear options between "Plain text" and "Tags" display modes.
1017-1024: LGTM! Clear solidColor toggle with helpful tooltip.The tooltip clearly explains the visual difference between solid and subtle tag styles.
884-1238: LGTM! Comprehensive tag styling configuration.The extensive tag styling options provide excellent customization capabilities while maintaining good organization through collapsible panels.
| .ant-tag { | ||
| align-content: center; | ||
| cursor: default !important; | ||
| pointer-events: none !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hover disables pointer events on all nested tags – may break close icon
pointer-events:none on .ant-tag during hover prevents the built-in close (‘x’) button from receiving clicks in multi-select mode. If read-only behaviour is desired, guard it with a prop instead of unconditional CSS.
Consider:
.ant-tag[disabled] { pointer-events:none; }🤖 Prompt for AI Agents
In shesha-reactjs/src/components/dropdown/style.ts around lines 33 to 38, the
CSS rule unconditionally sets pointer-events:none on .ant-tag during hover,
which disables interaction with the close icon in multi-select mode. To fix
this, remove the unconditional pointer-events:none and instead apply it
conditionally using a prop or attribute, such as .ant-tag[disabled] {
pointer-events:none; }, so that pointer events are only disabled when explicitly
desired.
| import { CSSProperties } from 'react'; | ||
|
|
||
| export const useStyles = createStyles(({ css, cx, token }, { style, tagStyle }: { style: CSSProperties; tagStyle: CSSProperties }) => { | ||
| export const useStyles = createStyles(({ css, cx, token }, { style }: { style: CSSProperties }) => { |
There was a problem hiding this comment.
Mismatch between CSS variable names – font weight will not apply
You set
--ant-font-weight-strong: ${style.fontWeight};
Later, .ant-select-selection-item uses font-weight: var(--ant-font-weight); – note the missing “-strong”. This leaves the weight uncontrolled.
- --ant-font-weight-strong: ${style.fontWeight} !important;
+ --ant-font-weight: ${style.fontWeight} !important;Apply the same rename (or update the consumer) to restore the intended styling.
📝 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.
| export const useStyles = createStyles(({ css, cx, token }, { style }: { style: CSSProperties }) => { | |
| export const useStyles = createStyles( | |
| ({ css, cx, token }, { style }: { style: CSSProperties }) => ({ | |
| dropdown: css` | |
| - --ant-font-weight-strong: ${style.fontWeight} !important; | |
| + --ant-font-weight: ${style.fontWeight} !important; | |
| /* …other rules… */ | |
| `, | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/dropdown/style.ts at line 4, the CSS variable
for font weight is defined as --ant-font-weight-strong but the style uses
--ant-font-weight, causing the font weight not to apply. To fix this, rename the
CSS variable to --ant-font-weight to match the usage in
.ant-select-selection-item or update the consumer to use
--ant-font-weight-strong consistently, ensuring the font weight is correctly
applied.
shesha-reactjs/src/components/refListDropDown/genericRefListDropDown.tsx
Show resolved
Hide resolved
shesha-reactjs/src/components/refListDropDown/genericRefListDropDown.tsx
Outdated
Show resolved
Hide resolved
| import { getTagStyle } from '@/utils/style'; | ||
| import convertCssColorNameToHex from 'convert-css-color-name-to-hex'; | ||
| import { Tag, Tooltip, TooltipProps } from 'antd'; | ||
| import React from 'react'; | ||
| import { Icon } from '../readOnlyDisplayFormItem'; | ||
| function ReflistTag({ value, tooltip, color, icon, showIcon, tagStyle, solidColor, showItemName, label, placement = 'right' }) { | ||
|
|
||
| const memoizedColor = solidColor | ||
| ? convertCssColorNameToHex(color ?? '') | ||
| : color?.toLowerCase(); | ||
|
|
||
| return ( | ||
| <Tooltip title={tooltip} placement={placement as TooltipProps['placement']} style={{ cursor: 'pointer' }}> | ||
| <Tag | ||
| key={value} | ||
| color={memoizedColor} | ||
| icon={(icon && showIcon) && <Icon type={icon} />} | ||
| style={getTagStyle(tagStyle, !!color)} | ||
| >{showItemName && label}</Tag> | ||
| </Tooltip> | ||
| ); | ||
| }; | ||
|
|
||
| export default ReflistTag; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript interface for component props to improve type safety.
The component lacks TypeScript typing, which reduces type safety and IDE support. As this is a reusable component used across multiple files, proper typing is essential.
Add TypeScript interface and type the component:
import { getTagStyle } from '@/utils/style';
import convertCssColorNameToHex from 'convert-css-color-name-to-hex';
import { Tag, Tooltip, TooltipProps } from 'antd';
-import React from 'react';
+import React, { FC, ReactNode } from 'react';
import { Icon } from '../readOnlyDisplayFormItem';
+
+interface IReflistTagProps {
+ value: string | number;
+ tooltip?: string;
+ color?: string;
+ icon?: string;
+ showIcon?: boolean;
+ tagStyle?: React.CSSProperties;
+ solidColor?: boolean;
+ showItemName?: boolean;
+ label?: ReactNode;
+ placement?: TooltipProps['placement'];
+}
+
-function ReflistTag({ value, tooltip, color, icon, showIcon, tagStyle, solidColor, showItemName, label, placement = 'right' }) {
+const ReflistTag: FC<IReflistTagProps> = ({
+ value,
+ tooltip,
+ color,
+ icon,
+ showIcon,
+ tagStyle,
+ solidColor,
+ showItemName,
+ label,
+ placement = 'right'
+}) => {
const memoizedColor = solidColor
? convertCssColorNameToHex(color ?? '')
: color?.toLowerCase();
return (
<Tooltip title={tooltip} placement={placement as TooltipProps['placement']} style={{ cursor: 'pointer' }}>
<Tag
key={value}
color={memoizedColor}
icon={(icon && showIcon) && <Icon type={icon} />}
style={getTagStyle(tagStyle, !!color)}
>{showItemName && label}</Tag>
</Tooltip>
);
};
export default ReflistTag;📝 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.
| import { getTagStyle } from '@/utils/style'; | |
| import convertCssColorNameToHex from 'convert-css-color-name-to-hex'; | |
| import { Tag, Tooltip, TooltipProps } from 'antd'; | |
| import React from 'react'; | |
| import { Icon } from '../readOnlyDisplayFormItem'; | |
| function ReflistTag({ value, tooltip, color, icon, showIcon, tagStyle, solidColor, showItemName, label, placement = 'right' }) { | |
| const memoizedColor = solidColor | |
| ? convertCssColorNameToHex(color ?? '') | |
| : color?.toLowerCase(); | |
| return ( | |
| <Tooltip title={tooltip} placement={placement as TooltipProps['placement']} style={{ cursor: 'pointer' }}> | |
| <Tag | |
| key={value} | |
| color={memoizedColor} | |
| icon={(icon && showIcon) && <Icon type={icon} />} | |
| style={getTagStyle(tagStyle, !!color)} | |
| >{showItemName && label}</Tag> | |
| </Tooltip> | |
| ); | |
| }; | |
| export default ReflistTag; | |
| import { getTagStyle } from '@/utils/style'; | |
| import convertCssColorNameToHex from 'convert-css-color-name-to-hex'; | |
| import { Tag, Tooltip, TooltipProps } from 'antd'; | |
| import React, { FC, ReactNode } from 'react'; | |
| import { Icon } from '../readOnlyDisplayFormItem'; | |
| interface IReflistTagProps { | |
| value: string | number; | |
| tooltip?: string; | |
| color?: string; | |
| icon?: string; | |
| showIcon?: boolean; | |
| tagStyle?: React.CSSProperties; | |
| solidColor?: boolean; | |
| showItemName?: boolean; | |
| label?: ReactNode; | |
| placement?: TooltipProps['placement']; | |
| } | |
| const ReflistTag: FC<IReflistTagProps> = ({ | |
| value, | |
| tooltip, | |
| color, | |
| icon, | |
| showIcon, | |
| tagStyle, | |
| solidColor, | |
| showItemName, | |
| label, | |
| placement = 'right', | |
| }) => { | |
| const memoizedColor = solidColor | |
| ? convertCssColorNameToHex(color ?? '') | |
| : color?.toLowerCase(); | |
| return ( | |
| <Tooltip title={tooltip} placement={placement as TooltipProps['placement']} style={{ cursor: 'pointer' }}> | |
| <Tag | |
| key={value} | |
| color={memoizedColor} | |
| icon={(icon && showIcon) && <Icon type={icon} />} | |
| style={getTagStyle(tagStyle, !!color)} | |
| > | |
| {showItemName && label} | |
| </Tag> | |
| </Tooltip> | |
| ); | |
| }; | |
| export default ReflistTag; |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/refListDropDown/reflistTag.tsx lines 1 to 24,
the ReflistTag component lacks TypeScript typings for its props, reducing type
safety and IDE support. Define a TypeScript interface describing all expected
props with their types, including optional and defaulted ones, then update the
component function signature to use this interface for its props parameter.
| return <ReflistTag | ||
| showIcon={showIcon} | ||
| showItemName={showItemName} | ||
| tagStyle={tagStyle} | ||
| solidColor={solidColor} | ||
| label={option?.label} | ||
| value={props.value} | ||
| key={props.value} | ||
| tooltip={"multiple"} | ||
| color={option?.color} | ||
| icon={option?.icon && showIcon && <Icon type={option?.icon} />} | ||
| style={getTagStyle(tagStyle, !!option?.color && solidColor)} | ||
| >{showItemName && option?.label}</Tag>; | ||
| ></ReflistTag>; |
There was a problem hiding this comment.
Fix hardcoded tooltip and inconsistent Icon handling.
Two issues:
- The hardcoded tooltip "multiple" doesn't provide useful information
- Icon is being wrapped in Icon component here, but ReflistTag already handles this internally
return <ReflistTag
showIcon={showIcon}
showItemName={showItemName}
tagStyle={tagStyle}
solidColor={solidColor}
label={option?.label}
value={props.value}
key={props.value}
- tooltip={"multiple"}
+ tooltip={option?.description}
color={option?.color}
- icon={option?.icon && showIcon && <Icon type={option?.icon} />}
+ icon={option?.icon}
></ReflistTag>;📝 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.
| return <ReflistTag | |
| showIcon={showIcon} | |
| showItemName={showItemName} | |
| tagStyle={tagStyle} | |
| solidColor={solidColor} | |
| label={option?.label} | |
| value={props.value} | |
| key={props.value} | |
| tooltip={"multiple"} | |
| color={option?.color} | |
| icon={option?.icon && showIcon && <Icon type={option?.icon} />} | |
| style={getTagStyle(tagStyle, !!option?.color && solidColor)} | |
| >{showItemName && option?.label}</Tag>; | |
| ></ReflistTag>; | |
| return <ReflistTag | |
| showIcon={showIcon} | |
| showItemName={showItemName} | |
| tagStyle={tagStyle} | |
| solidColor={solidColor} | |
| label={option?.label} | |
| value={props.value} | |
| key={props.value} | |
| tooltip={option?.description} | |
| color={option?.color} | |
| icon={option?.icon} | |
| ></ReflistTag>; |
🤖 Prompt for AI Agents
In shesha-reactjs/src/components/dropdown/dropdown.tsx around lines 226 to 237,
the tooltip prop is hardcoded to "multiple" which is not informative; replace it
with a dynamic or contextually relevant value. Also, remove the wrapping of
option?.icon inside the Icon component since ReflistTag already manages icon
rendering internally, so just pass the icon data directly without wrapping.
2 participants
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Refactor