-
Notifications
You must be signed in to change notification settings - Fork 557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add PillBadgeView to Python Panels #4909
Changes from 16 commits
248c341
4193b4f
15eadbe
8b30e1c
61623ca
cea0eba
d1c58a7
e95592b
030fb8b
9538c5b
c9f3dd4
1646a62
c672a05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,140 @@ | ||||||||||||||||||||||||||||||||||||||
import React, { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||
import CircleIcon from "@mui/icons-material/Circle"; | ||||||||||||||||||||||||||||||||||||||
import { Chip, FormControl, MenuItem, Select } from "@mui/material"; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
const PillBadge = ({ | ||||||||||||||||||||||||||||||||||||||
text, | ||||||||||||||||||||||||||||||||||||||
color = "default", | ||||||||||||||||||||||||||||||||||||||
variant = "filled", | ||||||||||||||||||||||||||||||||||||||
showIcon = true, | ||||||||||||||||||||||||||||||||||||||
}: { | ||||||||||||||||||||||||||||||||||||||
text: string | string[] | [string, string][]; | ||||||||||||||||||||||||||||||||||||||
color?: string; | ||||||||||||||||||||||||||||||||||||||
variant?: "filled" | "outlined"; | ||||||||||||||||||||||||||||||||||||||
showIcon?: boolean; | ||||||||||||||||||||||||||||||||||||||
}) => { | ||||||||||||||||||||||||||||||||||||||
const [chipSelection, setChipSelection] = useState( | ||||||||||||||||||||||||||||||||||||||
typeof text === "string" | ||||||||||||||||||||||||||||||||||||||
? text | ||||||||||||||||||||||||||||||||||||||
: Array.isArray(text) | ||||||||||||||||||||||||||||||||||||||
? Array.isArray(text[0]) | ||||||||||||||||||||||||||||||||||||||
? text[0][0] | ||||||||||||||||||||||||||||||||||||||
: text[0] | ||||||||||||||||||||||||||||||||||||||
: "" | ||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||
const [chipColor, setChipColor] = useState( | ||||||||||||||||||||||||||||||||||||||
typeof text === "string" | ||||||||||||||||||||||||||||||||||||||
? color | ||||||||||||||||||||||||||||||||||||||
: Array.isArray(text) | ||||||||||||||||||||||||||||||||||||||
? Array.isArray(text[0]) | ||||||||||||||||||||||||||||||||||||||
? text[0][1] | ||||||||||||||||||||||||||||||||||||||
: color || "default" | ||||||||||||||||||||||||||||||||||||||
: "default" | ||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||
Br2850 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
const COLORS: { [key: string]: string } = { | ||||||||||||||||||||||||||||||||||||||
default: "#999999", | ||||||||||||||||||||||||||||||||||||||
primary: "#FFB682", | ||||||||||||||||||||||||||||||||||||||
error: "error", | ||||||||||||||||||||||||||||||||||||||
warning: "warning", | ||||||||||||||||||||||||||||||||||||||
info: "info", | ||||||||||||||||||||||||||||||||||||||
success: "#8BC18D", | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+44
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure In the For example, you can update the const COLORS: { [key: string]: string } = {
default: "#999999",
primary: "#FFB682",
- error: "error",
- warning: "warning",
- info: "info",
+ error: "#f44336",
+ warning: "#ff9800",
+ info: "#2196f3",
success: "#8BC18D",
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
const chipStyle: { [key: string]: string | number } = { | ||||||||||||||||||||||||||||||||||||||
color: COLORS[chipColor || "default"] || COLORS.default, | ||||||||||||||||||||||||||||||||||||||
fontWeight: 500, | ||||||||||||||||||||||||||||||||||||||
paddingLeft: 1, | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||
<span> | ||||||||||||||||||||||||||||||||||||||
{typeof text === "string" ? ( | ||||||||||||||||||||||||||||||||||||||
<Chip | ||||||||||||||||||||||||||||||||||||||
icon={ | ||||||||||||||||||||||||||||||||||||||
showIcon ? ( | ||||||||||||||||||||||||||||||||||||||
<CircleIcon color={"inherit"} sx={{ fontSize: 10 }} /> | ||||||||||||||||||||||||||||||||||||||
) : undefined | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
label={text} | ||||||||||||||||||||||||||||||||||||||
sx={{ | ||||||||||||||||||||||||||||||||||||||
...chipStyle, | ||||||||||||||||||||||||||||||||||||||
"& .MuiChip-icon": { | ||||||||||||||||||||||||||||||||||||||
marginRight: "-7px", | ||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||
"& .MuiChip-label": { | ||||||||||||||||||||||||||||||||||||||
marginBottom: "1px", | ||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||
variant={variant as "filled" | "outlined" | undefined} | ||||||||||||||||||||||||||||||||||||||
/> | ||||||||||||||||||||||||||||||||||||||
) : ( | ||||||||||||||||||||||||||||||||||||||
<FormControl fullWidth> | ||||||||||||||||||||||||||||||||||||||
<Chip | ||||||||||||||||||||||||||||||||||||||
icon={ | ||||||||||||||||||||||||||||||||||||||
showIcon ? ( | ||||||||||||||||||||||||||||||||||||||
<CircleIcon color={"inherit"} sx={{ fontSize: 10 }} /> | ||||||||||||||||||||||||||||||||||||||
) : undefined | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
label={ | ||||||||||||||||||||||||||||||||||||||
Array.isArray(text) && Array.isArray(text[0]) ? ( | ||||||||||||||||||||||||||||||||||||||
<Select | ||||||||||||||||||||||||||||||||||||||
value={chipSelection} | ||||||||||||||||||||||||||||||||||||||
variant={"standard"} | ||||||||||||||||||||||||||||||||||||||
disableUnderline={true} | ||||||||||||||||||||||||||||||||||||||
onChange={(event) => { | ||||||||||||||||||||||||||||||||||||||
const selectedText = text.find( | ||||||||||||||||||||||||||||||||||||||
(t) => t[0] === event.target.value | ||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||
setChipSelection(event.target.value); | ||||||||||||||||||||||||||||||||||||||
setChipColor(selectedText?.[1] ?? "default"); | ||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+94
to
+100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add checks for In the Here's an example of how to adjust the code: onChange={(event) => {
const selectedText = text.find(
(t) => t[0] === event.target.value
);
setChipSelection(event.target.value);
- setChipColor(selectedText?.[1] ?? "default");
+ if (selectedText) {
+ setChipColor(selectedText[1]);
+ } else {
+ setChipColor("default");
+ }
}} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
sx={{ | ||||||||||||||||||||||||||||||||||||||
color: "inherit", | ||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||
{text.map((t, index) => ( | ||||||||||||||||||||||||||||||||||||||
<MenuItem key={index} value={t[0]}> | ||||||||||||||||||||||||||||||||||||||
{t[0]} | ||||||||||||||||||||||||||||||||||||||
</MenuItem> | ||||||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||||||
</Select> | ||||||||||||||||||||||||||||||||||||||
Br2850 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
) : ( | ||||||||||||||||||||||||||||||||||||||
<Select | ||||||||||||||||||||||||||||||||||||||
value={chipSelection} | ||||||||||||||||||||||||||||||||||||||
variant={"standard"} | ||||||||||||||||||||||||||||||||||||||
disableUnderline={true} | ||||||||||||||||||||||||||||||||||||||
onChange={(event) => setChipSelection(event.target.value)} | ||||||||||||||||||||||||||||||||||||||
sx={{ | ||||||||||||||||||||||||||||||||||||||
color: "inherit", | ||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||||
{text.map((t, index) => ( | ||||||||||||||||||||||||||||||||||||||
<MenuItem key={index} value={t}> | ||||||||||||||||||||||||||||||||||||||
{t} | ||||||||||||||||||||||||||||||||||||||
</MenuItem> | ||||||||||||||||||||||||||||||||||||||
))} | ||||||||||||||||||||||||||||||||||||||
</Select> | ||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
sx={{ | ||||||||||||||||||||||||||||||||||||||
...chipStyle, | ||||||||||||||||||||||||||||||||||||||
"& .MuiChip-icon": { | ||||||||||||||||||||||||||||||||||||||
marginRight: "-7px", | ||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||
"& .MuiChip-label": { | ||||||||||||||||||||||||||||||||||||||
marginBottom: "1px", | ||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||
"& .MuiInput-input:focus": { | ||||||||||||||||||||||||||||||||||||||
backgroundColor: "inherit", | ||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||||
variant={variant as "filled" | "outlined" | undefined} | ||||||||||||||||||||||||||||||||||||||
></Chip> | ||||||||||||||||||||||||||||||||||||||
</FormControl> | ||||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||||
</span> | ||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
export default PillBadge; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default as PillBadge } from "./PillBadge"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { Box } from "@mui/material"; | ||
import React from "react"; | ||
import { getComponentProps } from "../utils"; | ||
import PillBadge from "@fiftyone/components/src/components/PillBadge/PillBadge"; | ||
|
||
export default function PillBadgeView(props) { | ||
const { schema } = props; | ||
const { view = {} } = schema; | ||
const { text, color, variant, showIcon } = view; | ||
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider adding TypeScript types and using a named export. While the component structure is good, it could benefit from the following improvements:
Here's a suggested refactor: import { Box } from "@mui/material";
import React from "react";
import { getComponentProps } from "../utils";
import PillBadge from "@fiftyone/components/src/components/PillBadge/PillBadge";
interface PillBadgeViewProps {
schema: {
view?: {
text?: string;
color?: string;
variant?: string;
showIcon?: boolean;
};
};
}
export function PillBadgeView({ schema }: PillBadgeViewProps) {
const { view = {} } = schema;
const { text, color, variant, showIcon } = view;
// ... rest of the component
} |
||
|
||
return ( | ||
<Box {...getComponentProps(props, "container")}> | ||
<PillBadge | ||
text={text} | ||
color={color} | ||
variant={variant} | ||
showIcon={showIcon} | ||
{...getComponentProps(props, "pillBadge")} | ||
/> | ||
</Box> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider simplifying the
text
prop to improve maintainability.The
text
prop accepts multiple types (string
,string[]
, and[string, string][]
), which increases complexity in the component logic and can lead to potential errors. Simplifying the prop to accept a single data type or splitting the component into separate components for each case could enhance readability and maintainability.