Update plugin components #4927
Update plugin components #4927
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![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
🧹 Outside diff range and nitpick comments (1)
fiftyone/operators/types.py (1)
Line range hint
256-308: LGTM! Consider updating the docstring.The addition of the
disabledparameter is a good improvement, allowing for more control over button states. The implementation looks correct.Consider updating the method's docstring to include information about the new
disabledparameter:def btn( self, name, label, icon=None, variant=None, disabled=False, on_click=None, prompt=False, params=None, space=None, href=None, icon_position="left", **kwargs, ): """Defines a button or icon button to display to the user as a :class:`Button`. ... Args: ... + disabled (False): whether the button is disabled variant (None): the variant of the button. Can be ``"contained"``, ``"outlined"``, ``"round"`` or ``"square"`` ... """
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- app/packages/components/src/components/PillBadge/PillBadge.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/SliderView.tsx (1 hunks)
- fiftyone/operators/types.py (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/packages/components/src/components/PillBadge/PillBadge.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/packages/core/src/plugins/SchemaIO/components/SliderView.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🪛 Biome
app/packages/core/src/plugins/SchemaIO/components/SliderView.tsx
[error] 212-212: Avoid the delete operator which can impact performance.
(lint/performance/noDelete)
🔇 Additional comments (1)
app/packages/core/src/plugins/SchemaIO/components/SliderView.tsx (1)
1-41: LGTM: Imports and type definitions are well-structured.The new imports from @mui/material and the additional type definitions (
ValueFormatandSliderInputFieldProps) are appropriate for the enhanced functionality of the SliderView component. The code structure in this segment is clean and follows TypeScript best practices.
Cherry-pick ModalView and relevant changes from tag-modal branch
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Outside diff range and nitpick comments (2)
app/packages/core/src/plugins/SchemaIO/components/ModalView.tsx (1)
1-30: Add component documentation and usage examples.As this component is part of the SchemaIO plugin system for ML panels, it should be well-documented.
Add JSDoc documentation:
/** * ModalView Component * * A SchemaIO wrapper around ModalBase for use in ML panels. * * @example * ```tsx * <ModalView * schema={{ * view: { * modal: { title: "ML Panel" }, * primaryButton: { text: "Run" }, * primaryCallback: () => handleRun() * } * }} * /> * ``` */app/packages/components/src/components/ModalBase/DisplayTags.tsx (1)
80-80: Consider using named export for better tree-shaking.Using named exports can help with tree-shaking and make imports more explicit.
-export default DisplayTags; +export { DisplayTags };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- app/packages/components/src/components/ModalBase/DisplayTags.tsx (1 hunks)
- app/packages/components/src/components/ModalBase/ModalBase.tsx (1 hunks)
- app/packages/components/src/components/ModalBase/index.ts (1 hunks)
- app/packages/components/src/components/MuiIconFont/index.tsx (1 hunks)
- app/packages/components/src/components/PillBadge/PillBadge.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/ModalView.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/PillBadgeView.tsx (1 hunks)
- app/packages/core/src/plugins/SchemaIO/components/index.ts (2 hunks)
- app/packages/operators/src/types.ts (2 hunks)
- fiftyone/operators/types.py (7 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/packages/components/src/components/ModalBase/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/packages/components/src/components/PillBadge/PillBadge.tsx
- app/packages/core/src/plugins/SchemaIO/components/PillBadgeView.tsx
- app/packages/core/src/plugins/SchemaIO/components/index.ts
- app/packages/operators/src/types.ts
🧰 Additional context used
📓 Path-based instructions (4)
app/packages/components/src/components/ModalBase/DisplayTags.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/components/src/components/ModalBase/ModalBase.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/components/src/components/MuiIconFont/index.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/core/src/plugins/SchemaIO/components/ModalView.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🪛 Biome
app/packages/components/src/components/ModalBase/ModalBase.tsx
[error] 161-163: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (6)
app/packages/components/src/components/MuiIconFont/index.tsx (1)
35-36: Type definition looks good!The type definition follows TypeScript best practices with:
- Proper type inheritance from IconProps
- Clear string literal union type for variants
- Optional variant prop
app/packages/components/src/components/ModalBase/DisplayTags.tsx (1)
1-80: Verify integration with ModalBase component.Let's ensure this component is properly integrated with ModalBase as mentioned in the summary.
✅ Verification successful
DisplayTags component is properly integrated with ModalBase
The verification shows that:
- DisplayTags is correctly imported in ModalBase.tsx using
import DisplayTags from "./DisplayTags"- It's properly used within ModalBase when the functionality is "tagging":
<DisplayTags saveTags={handleSaveTags} />- The component is rendered conditionally based on the functionality prop
- The required
saveTagsprop is being passed throughhandleSaveTags🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check how DisplayTags is imported and used in ModalBase ast-grep --pattern 'import { DisplayTags } from "./DisplayTags"' # Look for usage of DisplayTags component in ModalBase ast-grep --pattern '<DisplayTags $$$>'Length of output: 107
Script:
#!/bin/bash # Let's try a broader search for DisplayTags usage and imports rg -l "DisplayTags" --type tsx --type ts # Search for any file that contains both "ModalBase" and "DisplayTags" rg -l "DisplayTags.*ModalBase|ModalBase.*DisplayTags" --type tsx --type ts # Look for files in the ModalBase directory fd . "app/packages/components/src/components/ModalBase" # Check content of any ModalBase related files rg "DisplayTags" "app/packages/components/src/components/ModalBase" -A 2 -B 2Length of output: 2871
app/packages/components/src/components/ModalBase/ModalBase.tsx (3)
230-235: Verify theonClickhandlers forButtonViewto ensure proper event handling.Passing
handleCloseas theonClickprop while also havingonClickdefined withinsecondaryButtonViewmay lead to conflicts or unintended behavior.Review how the
ButtonViewcomponent handles these props to ensure the correctonClickevent is triggered. Confirm whether the externalonClickprop overrides or complements the internalview.onClick.
240-245: Verify theonClickhandlers forButtonViewto ensure proper event handling.Similar to the
secondaryButton, ensure that theonClickevents for the primary button are set up correctly to prevent any conflicts.Check the
ButtonViewimplementation to confirm the handling of multipleonClickdefinitions.
114-117:⚠️ Potential issueEnsure
propsis defined before accessing its properties.Accessing
propswithout checking if it's defined can lead to runtime errors ifpropsisundefinedornull.Modify the condition to safely access
props:-if (Object.keys(props).includes("icon")) { +if (props && Object.keys(props).includes("icon")) {This check prevents errors when
propsis not provided.Likely invalid or redundant comment.
fiftyone/operators/types.py (1)
Line range hint
256-300: Thedisabledparameter is correctly implemented inbtnmethodThe
disabledparameter is appropriately added to thebtnmethod's signature, documented in the docstring, and passed to theButtonconstructor within the method. This allows buttons to be created in a disabled state, enhancing the flexibility of the UI components.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- app/packages/components/src/components/ModalBase/ModalBase.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/packages/components/src/components/ModalBase/ModalBase.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🪛 Biome
app/packages/components/src/components/ModalBase/ModalBase.tsx
[error] 165-167: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (1)
app/packages/components/src/components/ModalBase/ModalBase.tsx (1)
168-179: 🛠️ Refactor suggestionOptimize
useEffectdependencies to prevent unnecessary re-renders.The
useEffecthook depends on objects (primaryButtonView,primaryButtonView.params), which may cause the effect to run on every render due to reference changes in these objects.Consider updating the dependencies to include only the necessary properties:
}, [ functionality, - primaryButtonView, - primaryButtonView.params + primaryButtonView.disabled ]);By specifying
primaryButtonView.disabled, the effect will only run when thedisabledstate changes, improving performance by preventing unnecessary re-executions.Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- app/packages/core/src/plugins/SchemaIO/components/GridView.tsx (3 hunks)
- fiftyone/operators/types.py (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/packages/core/src/plugins/SchemaIO/components/GridView.tsx
🧰 Additional context used
🔇 Additional comments (5)
fiftyone/operators/types.py (5)
Line range hint
256-300: LGTM: Button disabled state implementationThe addition of the
disabledparameter to the button implementation is well-documented and properly integrated into the existing codebase.
412-414: Documentation added for unimplemented featureThe documentation for the
iconparameter has been added, but based on past review comments, this feature is not yet implemented in the code.
1476-1485: LoadingView parameters need implementationThe LoadingView class has been documented with new parameters (
text,variant,color,size) but lacks their implementation.
1486-1493: PillBadgeView implementation missingThe PillBadgeView class has been added with comprehensive documentation but lacks implementation of its documented parameters.
2462-2463: MenuView icon parameter documentation without implementationThe icon parameter is documented but not implemented in the MenuView class.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (2)
app/packages/components/src/components/MuiIconFont/index.tsx (2)
10-10: Consider inlining the default propSince
defaultPropsis only used once in the component, consider inlining it directly in the destructuring:-const defaultProps = { variant: "filled" as const }; -const { name, variant = defaultProps.variant, ...iconProps } = props; +const { name, variant = "filled", ...iconProps } = props;
12-30: LGTM! Clean implementation with performance optimizationThe implementation is well-structured with proper memoization and clean variant mapping. The documentation link is helpful for users.
Consider simplifying the className handling:
-<Icon {...iconProps} className={className || undefined}> +<Icon {...iconProps} className={className || null}>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- app/packages/components/src/components/ModalBase/DisplayTags.tsx (1 hunks)
- app/packages/components/src/components/MuiIconFont/index.tsx (1 hunks)
- fiftyone/operators/types.py (7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/packages/components/src/components/ModalBase/DisplayTags.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/components/src/components/MuiIconFont/index.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🔇 Additional comments (5)
app/packages/components/src/components/MuiIconFont/index.tsx (1)
5-8: LGTM! Well-defined TypeScript typesThe type definition is clean and follows TypeScript best practices with proper union types for variants.
fiftyone/operators/types.py (4)
Line range hint
256-286: LGTM: Button disabled state implementationThe addition of the
disabledparameter is well-implemented with proper documentation and constructor propagation.
412-414: Implementation needed for documented icon parameterThe
iconparameter is documented but not implemented in the code.
1476-1485: Implementation needed for LoadingView parametersThe new parameters (
text,variant,color,size) are documented but not implemented in the__init__method.
1486-1493: Implementation needed for PillBadgeView parametersThe class is well-documented but the parameters are not implemented in the
__init__method.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
fiftyone/operators/types.py (1)
412-414: Minor: Improve docstring formattingThe
iconparameter documentation should maintain consistent indentation with other parameters in the docstring.Apply this formatting fix:
- icon (None): when set, the icon will be displayed as the menu button instead of the label. - Can be "SettingsIcon", "MoreVertIcon". + icon (None): when set, the icon will be displayed as the menu button instead of the + label. Can be "SettingsIcon" or "MoreVertIcon"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- app/packages/core/src/plugins/SchemaIO/components/index.ts (3 hunks)
- app/packages/operators/src/types.ts (3 hunks)
- fiftyone/operators/types.py (8 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
app/packages/core/src/plugins/SchemaIO/components/index.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.app/packages/operators/src/types.ts (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🔇 Additional comments (8)
app/packages/core/src/plugins/SchemaIO/components/index.ts (1)
36-36: LGTM! New exports are properly organized.The new exports (
ModalView,PillBadgeView, andTextView) are correctly placed following alphabetical ordering.Also applies to: 40-40, 51-51
app/packages/operators/src/types.ts (4)
1114-1126: LGTM! TextView implementation follows the established pattern.The TextView class is well-structured and consistent with other view implementations in the codebase.
1171-1183: LGTM! PillBadgeView implementation is correct.The PillBadgeView class properly extends the View class and maintains consistency with other view implementations.
1185-1197: LGTM! ModalView extends Button appropriately.The ModalView class correctly extends the Button class, which is appropriate since modals are typically triggered by button interactions.
1265-1269: LGTM! VIEWS constant updated correctly.The new view classes (TextView, PillBadgeView, ModalView) are properly registered in the VIEWS constant.
fiftyone/operators/types.py (3)
Line range hint
256-308: LGTM: Button disabled state implementationThe addition of the
disabledparameter to thebtnmethod is well-implemented and properly documented. The parameter follows the existing pattern and is correctly passed to the Button constructor.
1889-1897: LGTM: TextView implementationThe TextView class is properly implemented as a basic text display component with appropriate inheritance and documentation.
2368-2405:⚠️ Potential issueEnhance ModalView implementation
The ModalView class needs proper handling of its parameters (
modal,primaryButton,secondaryButton, etc.) and should include parameter validation.Apply this implementation:
def __init__(self, **kwargs): + self.modal = kwargs.get("modal", {}) + self.primaryButton = kwargs.get("primaryButton", {}) + self.secondaryButton = kwargs.get("secondaryButton", None) + self.functionality = kwargs.get("functionality", None) + + if not isinstance(self.modal, dict): + raise ValueError("The 'modal' parameter must be a dictionary") + if not isinstance(self.primaryButton, dict): + raise ValueError("The 'primaryButton' parameter must be a dictionary") super().__init__(**kwargs) +def to_json(self): + return { + **super().to_json(), + "modal": self.modal, + "primaryButton": self.primaryButton, + "secondaryButton": self.secondaryButton, + "functionality": self.functionality, + }Likely invalid or redundant comment.
| text ("Loading"): a label for the loading indicator | ||
| variant ("spinner"): the variant of the loading indicator | ||
| color ("primary"): the color of the loading indicator | ||
| size ("medium"): the size of the loading indicator | ||
| """ | ||
|
|
||
| def __init__(self, **kwargs): | ||
| super().__init__(**kwargs) | ||
|
|
||
|
|
There was a problem hiding this comment.
Implementation needed for documented parameters
The docstring documents new parameters (text, variant, color, size) but they are not implemented in the __init__ method.
Apply this implementation:
def __init__(self, **kwargs):
+ self.text = kwargs.get("text", "Loading")
+ self.variant = kwargs.get("variant", "spinner")
+ self.color = kwargs.get("color", "primary")
+ self.size = kwargs.get("size", "medium")
super().__init__(**kwargs)
+def to_json(self):
+ return {
+ **super().to_json(),
+ "text": self.text,
+ "variant": self.variant,
+ "color": self.color,
+ "size": self.size,
+ }Committable suggestion was skipped due to low confidence.
| class PillBadgeView(View): | ||
| """Displays a pill shaped badge. | ||
|
|
||
| Args: | ||
| text ("Reviewed" | ["Reviewed", "Not Reviewed"] | [["Not Started", "primary"], ["Reviewed", "success"], ["In Review", "warning"]): a label or set of label options with or without a color for the pill badge | ||
| color ("primary"): the color of the pill | ||
| variant ("outlined"): the variant of the pill | ||
| show_icon (False | True): whether to display indicator icon |
There was a problem hiding this comment.
Implementation needed for PillBadgeView parameters
The class documentation describes parameters (text, color, variant, show_icon) but they are not implemented.
Apply this implementation:
def __init__(self, **kwargs):
+ self.text = kwargs.get("text", "Reviewed")
+ self.color = kwargs.get("color", "primary")
+ self.variant = kwargs.get("variant", "outlined")
+ self.show_icon = kwargs.get("show_icon", False)
super().__init__(**kwargs)
+def to_json(self):
+ return {
+ **super().to_json(),
+ "text": self.text,
+ "color": self.color,
+ "variant": self.variant,
+ "show_icon": self.show_icon,
+ }Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- app/packages/components/src/components/ModalBase/ModalBase.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/packages/components/src/components/ModalBase/ModalBase.tsx (1)
Pattern
**/*.{ts,tsx}: Review the Typescript and React code for conformity with best practices in React, Recoil, Graphql, and Typescript. Highlight any deviations.
🪛 Biome
app/packages/components/src/components/ModalBase/ModalBase.tsx
[error] 165-167: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.(lint/complexity/noUselessLoneBlockStatements)
| interface ModalButtonView { | ||
| variant: string; | ||
| label: string; | ||
| icon?: string; | ||
| iconPosition?: string; | ||
| componentsProps: any; | ||
| } |
There was a problem hiding this comment.
Enhance type safety in ModalButtonView interface.
The componentsProps property is typed as any, which reduces type safety. Consider defining a more specific type structure.
interface ModalButtonView {
variant: string;
label: string;
icon?: string;
iconPosition?: string;
- componentsProps: any;
+ componentsProps: {
+ button: {
+ sx: {
+ height?: string | number;
+ width?: string | number;
+ padding?: number;
+ };
+ };
+ };
}Committable suggestion was skipped due to low confidence.
| {modal?.icon && ( | ||
| <MuiIconFont | ||
| variant={modal?.iconVariant || "filled"} | ||
| sx={{ verticalAlign: "middle" }} | ||
| name={modal.icon} | ||
| > | ||
| {modal.icon} | ||
| </MuiIconFont> | ||
| )}{" "} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract icon rendering into a separate component.
The icon rendering logic could be extracted into a reusable component for better maintainability and reusability.
+const ModalIcon: React.FC<{ icon: string; variant?: string }> = ({
+ icon,
+ variant = "filled"
+}) => (
+ <MuiIconFont
+ variant={variant}
+ sx={{ verticalAlign: "middle" }}
+ name={icon}
+ >
+ {icon}
+ </MuiIconFont>
+);
-{modal?.icon && (
- <MuiIconFont
- variant={modal?.iconVariant || "filled"}
- sx={{ verticalAlign: "middle" }}
- name={modal.icon}
- >
- {modal.icon}
- </MuiIconFont>
-)}
+{modal?.icon && <ModalIcon icon={modal.icon} variant={modal.iconVariant} />}📝 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.
| {modal?.icon && ( | |
| <MuiIconFont | |
| variant={modal?.iconVariant || "filled"} | |
| sx={{ verticalAlign: "middle" }} | |
| name={modal.icon} | |
| > | |
| {modal.icon} | |
| </MuiIconFont> | |
| )}{" "} | |
| const ModalIcon: React.FC<{ icon: string; variant?: string }> = ({ | |
| icon, | |
| variant = "filled" | |
| }) => ( | |
| <MuiIconFont | |
| variant={variant} | |
| sx={{ verticalAlign: "middle" }} | |
| name={icon} | |
| > | |
| {icon} | |
| </MuiIconFont> | |
| ); | |
| {modal?.icon && <ModalIcon icon={modal.icon} variant={modal.iconVariant} />}{" "} |
| const modalStyle = { | ||
| position: "absolute", | ||
| top: "50%", | ||
| left: "50%", | ||
| transform: "translate(-50%, -50%)", | ||
| width: 600, | ||
| bgcolor: "background.paper", | ||
| border: "2px solid #000", | ||
| boxShadow: 24, | ||
| p: 6, // Padding for inner spacing | ||
| display: "flex", | ||
| flexDirection: "column", // Stack items vertically | ||
| justifyContent: "center", // Vertically center the content | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make modal styles more configurable through props.
The modal styles are currently hardcoded. Consider making them configurable through props to support different use cases.
+interface ModalStyleProps {
+ width?: number;
+ padding?: number;
+ border?: string;
+}
const modalStyle = {
position: "absolute",
top: "50%",
left: "50%",
transform: "translate(-50%, -50%)",
- width: 600,
+ width: props.modalStyle?.width ?? 600,
bgcolor: "background.paper",
- border: "2px solid #000",
+ border: props.modalStyle?.border ?? "2px solid #000",
boxShadow: 24,
- p: 6,
+ p: props.modalStyle?.padding ?? 6,
display: "flex",
flexDirection: "column",
justifyContent: "center",
};Committable suggestion was skipped due to low confidence.
| const defaultAlign = "left"; | ||
|
|
||
| let titleAlign = defaultAlign; | ||
| let subtitleAlign = defaultAlign; | ||
| let bodyAlign = defaultAlign; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a single state object for text alignment configuration.
The current implementation uses multiple variables for alignment. Consider consolidating them into a single state object for better maintainability.
- const defaultAlign = "left";
- let titleAlign = defaultAlign;
- let subtitleAlign = defaultAlign;
- let bodyAlign = defaultAlign;
+ const [alignments, setAlignments] = useState({
+ title: "left",
+ subtitle: "left",
+ body: "left"
+ });Committable suggestion was skipped due to low confidence.
| if ( | ||
| (functionality === "tagging" || functionality === "Tagging") && | ||
| (!primaryButtonView.params || | ||
| !primaryButtonView.params.tags || | ||
| primaryButtonView.params.tags.length === 0) | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use enum for functionality type instead of string comparison.
Using string comparison for functionality type is error-prone. Consider using an enum instead.
+enum ModalFunctionality {
+ TAGGING = "tagging",
+ NONE = "none"
+}
-if (functionality === "tagging" || functionality === "Tagging")
+if (functionality.toLowerCase() === ModalFunctionality.TAGGING)Committable suggestion was skipped due to low confidence.
What changes are proposed in this pull request?
Updated plugin components and added new Views in SchemaIO to support the ML panels
How is this patch tested? If it is not, please explain why.
(Details)
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
Release Notes
New Features
DisplayTagscomponent for managing and displaying dynamic lists of tags.ModalBaseandModalViewcomponents for customizable modal dialogs with flexible content and actions.PillBadgeandPillBadgeViewcomponents for displaying pill-shaped badges with customizable options.ButtonViewwith a newdisabledproperty for better interactivity control.DropdownViewto support dynamic icon imports.SliderViewfor improved input handling and value formatting.TextViewcomponent for rendering styled text elements.LoadingViewenhancements for flexible loading state displays.Bug Fixes
Headerto prevent empty displays.Chores
getDisabledColorsfor consistent disabled styling across components.