Skip to content
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

Update plugin components #4927

Merged
merged 43 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
741ace3
add spinner variant
Br2850 Sep 30, 2024
5d45615
allow to only show icon in dropdownview
lanzhenw Oct 3, 2024
8a36c84
update type and minor tweak
lanzhenw Oct 7, 2024
5bea4b7
style tweak and clean up
lanzhenw Oct 7, 2024
71bd61a
improved sliderview
manivoxel51 Oct 3, 2024
ab131b3
progress
manivoxel51 Oct 4, 2024
b589949
complete slider
manivoxel51 Oct 7, 2024
9c1a519
pillBadge outline
Br2850 Oct 8, 2024
9b02e46
coloring
Br2850 Oct 8, 2024
162f883
base Chip
Br2850 Oct 8, 2024
4827e44
add PillBadgeView to Python Panel types
Br2850 Oct 9, 2024
da99e2d
colored badges
Br2850 Oct 10, 2024
930acd6
badge with circle and color
Br2850 Oct 10, 2024
2f1bf5e
extend badge to include dropdowns
Br2850 Oct 10, 2024
e56ecc2
show/hide icon
Br2850 Oct 10, 2024
c3c70dc
final color changes and select
Br2850 Oct 10, 2024
d24a5ac
color matching on selection
Br2850 Oct 10, 2024
aacf655
make use state vars more readable
Br2850 Oct 10, 2024
62de88e
Update app/packages/components/src/components/PillBadge/PillBadge.tsx
Br2850 Oct 10, 2024
e3448f1
add debouncing and throttle to slider onchange event
lanzhenw Oct 11, 2024
6404f05
use onChangeComitted instead
lanzhenw Oct 14, 2024
81fe8f8
cleanup
lanzhenw Oct 14, 2024
fa57350
modify icon only dropdownview style
lanzhenw Oct 16, 2024
f377f13
gridView change
lanzhenw Oct 17, 2024
5894643
Update PillBadge.tsx
Br2850 Oct 15, 2024
ca1bccd
adds disabled state for various variants of the button view
manivoxel51 Oct 16, 2024
7771d0a
hook up .btn to new disabled state
Br2850 Oct 16, 2024
d1f0541
docstring additions for LoadingView
Br2850 Oct 16, 2024
0c8a8a6
refactor and fix disabled color when no color code provided
manivoxel51 Oct 16, 2024
3fd5ba8
Adds a customizable TextView component (#4942)
manivoxel51 Oct 18, 2024
ea56505
snake_case the TextView component
manivoxel51 Oct 18, 2024
b095575
Merge pull request #4948 from voxel51/bug/fix-casing-textview
manivoxel51 Oct 18, 2024
66daf79
minor improvements
Br2850 Oct 18, 2024
d1260b7
cherry-pick ModalView and relevant changes from tag-modal branch
Br2850 Oct 20, 2024
af9f0f7
Merge pull request #4951 from voxel51/patch/panel-tag-modal
Br2850 Oct 20, 2024
947731c
pill badge onChange patch
Br2850 Oct 22, 2024
cb7dfec
ModalView enhancement
Br2850 Oct 22, 2024
4972cbc
Merge branch 'develop' into panel
lanzhenw Oct 22, 2024
3d67489
review changes 1
Br2850 Oct 22, 2024
6f81d66
review changes 2
Br2850 Oct 22, 2024
dfb8681
Merge branch 'panel' of github.com:voxel51/fiftyone into panel
Br2850 Oct 22, 2024
c2c3fcc
testing
manivoxel51 Oct 22, 2024
a9fde0a
infinite loop fix
Br2850 Oct 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions app/packages/components/src/components/Loading/LoadingSpinner.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React from "react";

import { CircularProgress } from "@mui/material";

const LoadingSpinner = ({
color = "base",
size = "medium",
}: {
color?: string;
size?: string;
}) => {
const COLORS: { [key: string]: string } = {
base: "#FFC59B",
primary: "primary",
secondary: "secondary",
error: "error",
warning: "warning",
info: "info",
success: "success",
};
const SIZES: { [key: string]: string } = {
small: "1rem",
medium: "2rem",
large: "3rem",
};
return <CircularProgress sx={{ color: COLORS[color] }} size={SIZES[size]} />;
};

export default LoadingSpinner;
88 changes: 88 additions & 0 deletions app/packages/components/src/components/ModalBase/DisplayTags.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import React, { useState } from "react";
import { Box, Chip, TextField, IconButton } from "@mui/material";
import AddIcon from "@mui/icons-material/Add";

// Define the props interface for the DisplayTags component
interface DisplayTagsProps {
saveTags: (tags: string[]) => void; // saveTags is a function that accepts an array of strings and returns void
}
Br2850 marked this conversation as resolved.
Show resolved Hide resolved

const DisplayTags: React.FC<DisplayTagsProps> = ({ saveTags }) => {
const [chips, setChips] = useState<string[]>([]); // chips is an array of strings
const [inputValue, setInputValue] = useState<string>(""); // inputValue is a string

Br2850 marked this conversation as resolved.
Show resolved Hide resolved
const handleKeyPress = (event: React.KeyboardEvent) => {
if (event.key === "Enter") {
event.preventDefault();
handleAddChip();
}
};

const handleAddChip = () => {
if (inputValue.trim() !== "") {
const updatedChips = [...chips, inputValue];
setChips(updatedChips);
setInputValue("");
saveTags(updatedChips); // Call the saveTags function to save the new list of chips
}
};

const handleDeleteChip = (chipToDelete: string) => {
const updatedChips = chips.filter((chip) => chip !== chipToDelete);
setChips(updatedChips);
saveTags(updatedChips); // Call the saveTags function to save the updated list of chips
};
Br2850 marked this conversation as resolved.
Show resolved Hide resolved

Br2850 marked this conversation as resolved.
Show resolved Hide resolved
return (
<Box
sx={{
display: "flex",
flexDirection: "column",
gap: 2,
alignItems: "start",
paddingTop: 1,
paddingBottom: 1,
width: "100%", // Ensure the box takes full width
}}
>
<Box
sx={{
display: "flex",
alignItems: "center",
gap: 1,
width: "100%", // Ensure the inner box takes full width
}}
>
<TextField
variant="outlined"
label="Enter tag"
value={inputValue}
onChange={(e) => setInputValue(e.target.value)}
onKeyDown={handleKeyPress}
fullWidth // Make TextField take up the remaining width
/>
<IconButton onClick={handleAddChip} color="primary">
<AddIcon />
</IconButton>
</Box>

<Box
sx={{
display: "flex",
gap: 1,
flexWrap: "wrap",
}}
>
{chips.map((chip, index) => (
<Chip
key={index}
label={chip}
onDelete={() => handleDeleteChip(chip)}
/>
))}
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
</Box>
</Box>
);
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
};

export default DisplayTags;
270 changes: 270 additions & 0 deletions app/packages/components/src/components/ModalBase/ModalBase.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
import React, { useCallback, useEffect, useState } from "react";
import ButtonView from "@fiftyone/core/src/plugins/SchemaIO/components/ButtonView";
import { Box, Modal, Typography } from "@mui/material";
import DisplayTags from "./DisplayTags";
import { MuiIconFont } from "../index";

interface ModalBaseProps {
modal: {
icon?: string;
iconVariant?: "outlined" | "filled" | "rounded" | "sharp" | undefined;
title: string;
subtitle: string;
body: string;
textAlign?: string | { [key: string]: string };
};
primaryButton?: {
href?: any;
prompt?: any;
params?: any;
operator?: any;
align?: string;
width?: string;
onClick?: any;
disabled?: boolean;
primaryText: string;
primaryColor: string;
};
secondaryButton?: {
href?: any;
prompt?: any;
params?: any;
operator?: any;
align?: string;
width?: string;
onClick?: any;
disabled?: boolean;
secondaryText: string;
secondaryColor: string;
};
functionality?: string;
primaryCallback?: () => void;
secondaryCallback?: () => void;
props: any;
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
}

interface ModalButtonView {
variant: string;
label: string;
icon?: string;
iconPosition?: string;
componentsProps: any;
}
Comment on lines +46 to +52
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.


const ModalBase: React.FC<ModalBaseProps> = ({
modal,
primaryButton,
secondaryButton,
primaryCallback,
secondaryCallback,
functionality = "none",
props,
}) => {
const { title, subtitle, body } = modal;

const defaultAlign = "left";

let titleAlign = defaultAlign;
let subtitleAlign = defaultAlign;
let bodyAlign = defaultAlign;
Comment on lines +65 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.


const [open, setOpen] = useState(false);
const handleOpen = () => setOpen(true);
const handleClose = () => {
if (!secondaryCallback) {
setOpen(false);
}
};
Br2850 marked this conversation as resolved.
Show resolved Hide resolved

if (typeof modal?.textAlign === "string") {
titleAlign = subtitleAlign = bodyAlign = modal.textAlign;
} else {
titleAlign = modal?.textAlign?.title ?? defaultAlign;
subtitleAlign = modal?.textAlign?.subtitle ?? defaultAlign;
bodyAlign = modal?.textAlign?.body ?? defaultAlign;
}

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
};
Comment on lines +87 to +100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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 modalButtonView: ModalButtonView = {
variant: props?.variant || "outlined",
label: props?.label || "Open Modal",
componentsProps: {
button: {
sx: {
height: props?.height || "100%",
width: props?.width || "100%",
padding: 1,
},
},
},
};

if (Object.keys(props).includes("icon")) {
modalButtonView["icon"] = props["icon"];
modalButtonView["iconPosition"] = props?.iconPosition || "left";
}

const [primaryButtonView, setPrimaryButtonView] = useState({
variant: "contained",
color: primaryButton?.primaryColor,
label: primaryButton?.primaryText,
onClick: primaryButton?.onClick,
operator: primaryCallback || primaryButton?.operator,
params: primaryButton?.params,
href: primaryButton?.href,
prompt: primaryButton?.prompt,
disabled: primaryButton?.disabled,
componentsProps: {
button: {
sx: {
width: primaryButton?.width || "100%",
justifyContent: primaryButton?.align || "center",
...primaryButton,
},
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
},
},
});

const [secondaryButtonView, setSecondaryButtonView] = useState({
variant: "outlined",
color: secondaryButton?.secondaryColor,
label: secondaryButton?.secondaryText,
onClick: secondaryButton?.onClick,
operator: secondaryCallback || secondaryButton?.operator,
params: secondaryButton?.params,
href: secondaryButton?.href,
prompt: secondaryButton?.prompt,
disabled: secondaryButton?.disabled,
componentsProps: {
button: {
sx: {
width: primaryButton?.width || "100%",
justifyContent: primaryButton?.align || "center",
...secondaryButton,
},
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
},
},
});

// State options for functionality based on user input

{
/* TAGGING FUNCTIONALITY */
}
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
useEffect(() => {
if (
(functionality === "tagging" || functionality === "Tagging") &&
(!primaryButtonView.params ||
!primaryButtonView.params.tags ||
primaryButtonView.params.tags.length === 0)
) {
Comment on lines +169 to +174
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

setPrimaryButtonView({ ...primaryButtonView, disabled: true });
} else {
setPrimaryButtonView({ ...primaryButtonView, disabled: false });
}
}, [primaryButtonView.params]);

const handleSaveTags = useCallback((tags: string[]) => {
setPrimaryButtonView((prevButtonView) => ({
...prevButtonView,
params: { ...prevButtonView.params, tags }, // Add tags to existing params
}));
}, []);

return (
<>
<Box>
<ButtonView
onClick={handleOpen}
schema={{
view: modalButtonView,
}}
/>
</Box>
<Modal
open={open}
onClose={handleClose}
aria-labelledby="modal-title"
aria-describedby="modal-subtitle"
>
<Box sx={modalStyle}>
<Typography
id="modal-title"
variant="h5"
component="h5"
sx={{ textAlign: titleAlign }}
>
{modal?.icon && (
<MuiIconFont
variant={modal?.iconVariant || "filled"}
sx={{ verticalAlign: "middle" }}
name={modal.icon}
>
{modal.icon}
</MuiIconFont>
)}{" "}
Comment on lines +211 to +219
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

Suggested change
{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} />}{" "}

{title}
</Typography>
<Typography
id="modal-subtitle"
variant="h6"
component="h6"
sx={{ mt: 4, textAlign: subtitleAlign }}
>
{subtitle}
</Typography>
<Typography id="modal-body" sx={{ my: 1, textAlign: bodyAlign }}>
{body}
</Typography>
{(functionality === "tagging" || functionality === "Tagging") && (
<DisplayTags saveTags={handleSaveTags} />
)}
<Box
sx={{
display: "flex",
justifyContent: "space-between",
mt: 2,
gap: 3,
}}
>
{secondaryButton && (
<Box sx={{ flexGrow: 1 }}>
<ButtonView
onClick={handleClose}
schema={{
view: secondaryButtonView,
}}
/>
</Box>
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
)}
{primaryButton && (
<Box sx={{ flexGrow: 1 }}>
<ButtonView
onClick={handleClose}
schema={{
view: primaryButtonView,
}}
/>
</Box>
Br2850 marked this conversation as resolved.
Show resolved Hide resolved
)}
</Box>
</Box>
</Modal>
</>
);
};
export default ModalBase;
1 change: 1 addition & 0 deletions app/packages/components/src/components/ModalBase/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as ModalBase } from "./ModalBase";
Loading
Loading