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.

Sign up for GitHub

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

fix: moved dropdowns to chart component + added pending icon #5824

Merged
merged 3 commits into from
Oct 14, 2024
Merged

fix: moved dropdowns to chart component + added pending icon #5824

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c141450
fix: moved dropdowns to chart component + added pending icon
gakshita Oct 14, 2024
82a1313
fix: copy changes
gakshita Oct 14, 2024
e10c831
fix: review changes
gakshita Oct 14, 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
1 change: 1 addition & 0 deletions packages/ui/src/icons/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ export * from "./favorite-folder-icon";
export * from "./planned-icon";
export * from "./in-progress-icon";
export * from "./done-icon";
export * from "./pending-icon";
27 changes: 27 additions & 0 deletions packages/ui/src/icons/pending-icon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as React from "react";

import { ISvgIcons } from "./type";

export const PendingState: React.FC<ISvgIcons> = ({ width = "10", height = "11", className, color }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Set #455068 as the default color instead of hardcoding, so that we can override if required.

<svg
width={width}
height={height}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 3C7.02944 3 3 7.02944 3 12C3 16.9706 7.02944 21 12 21C16.9706 21 21 16.9706 21 12C21 7.02944 16.9706 3 12 3ZM1 12C1 5.92487 5.92487 1 12 1C18.0751 1 23 5.92487 23 12C23 18.0751 18.0751 23 12 23C5.92487 23 1 18.0751 1 12Z"
fill="#455068"
/>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 5C12.5523 5 13 5.44772 13 6V11.382L16.4472 13.1056C16.9412 13.3526 17.1414 13.9532 16.8944 14.4472C16.6474 14.9412 16.0468 15.1414 15.5528 14.8944L11.5528 12.8944C11.214 12.725 11 12.3788 11 12V6C11 5.44772 11.4477 5 12 5Z"
fill="#455068"
/>
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 making the icon color customizable.

The SVG implementation looks good and should correctly render a pending state icon. However, the fill color is currently hardcoded as "#455068". To improve the icon's reusability across different color schemes, consider using the color prop to set the fill color dynamically.

Here's a suggested modification:

-      fill="#455068"
+      fill={color || "#455068"}

Apply this change to both path elements (lines 18 and 24).

📝 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
<svg
width={width}
height={height}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 3C7.02944 3 3 7.02944 3 12C3 16.9706 7.02944 21 12 21C16.9706 21 21 16.9706 21 12C21 7.02944 16.9706 3 12 3ZM1 12C1 5.92487 5.92487 1 12 1C18.0751 1 23 5.92487 23 12C23 18.0751 18.0751 23 12 23C5.92487 23 1 18.0751 1 12Z"
fill="#455068"
/>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 5C12.5523 5 13 5.44772 13 6V11.382L16.4472 13.1056C16.9412 13.3526 17.1414 13.9532 16.8944 14.4472C16.6474 14.9412 16.0468 15.1414 15.5528 14.8944L11.5528 12.8944C11.214 12.725 11 12.3788 11 12V6C11 5.44772 11.4477 5 12 5Z"
fill="#455068"
/>
<svg
width={width}
height={height}
viewBox="0 0 24 24"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 3C7.02944 3 3 7.02944 3 12C3 16.9706 7.02944 21 12 21C16.9706 21 21 16.9706 21 12C21 7.02944 16.9706 3 12 3ZM1 12C1 5.92487 5.92487 1 12 1C18.0751 1 23 5.92487 23 12C23 18.0751 18.0751 23 12 23C5.92487 23 1 18.0751 1 12Z"
fill={color || "#455068"}
/>
<path
fill-rule="evenodd"
clip-rule="evenodd"
d="M12 5C12.5523 5 13 5.44772 13 6V11.382L16.4472 13.1056C16.9412 13.3526 17.1414 13.9532 16.8944 14.4472C16.6474 14.9412 16.0468 15.1414 15.5528 14.8944L11.5528 12.8944C11.214 12.725 11 12.3788 11 12V6C11 5.44772 11.4477 5 12 5Z"
fill={color || "#455068"}
/>

</svg>
);
103 changes: 74 additions & 29 deletions web/ce/components/cycles/analytics-sidebar/base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
import { FC, Fragment } from "react";
import { observer } from "mobx-react";
// plane ui
import { Loader } from "@plane/ui";
import { TCycleEstimateType } from "@plane/types";
import { EEstimateSystem } from "@plane/types/src/enums";
import { CustomSelect, Loader } from "@plane/ui";
// components
import ProgressChart from "@/components/core/sidebar/progress-chart";
import { validateCycleSnapshot } from "@/components/cycles";
import { cycleEstimateOptions, validateCycleSnapshot } from "@/components/cycles";
// helpers
import { getDate } from "@/helpers/date-time.helper";
// hooks
import { useCycle } from "@/hooks/store";
import { useCycle, useProjectEstimates } from "@/hooks/store";

type ProgressChartProps = {
workspaceSlug: string;
Expand All @@ -20,7 +22,9 @@ export const SidebarChart: FC<ProgressChartProps> = observer((props) => {
const { workspaceSlug, projectId, cycleId } = props;

// hooks
const { getEstimateTypeByCycleId, getCycleById } = useCycle();
const { getEstimateTypeByCycleId, getCycleById, fetchCycleDetails, fetchArchivedCycleDetails, setEstimateType } =
useCycle();
const { currentActiveEstimateId, areEstimateEnabledByProjectId, estimateById } = useProjectEstimates();

// derived data
const cycleDetails = validateCycleSnapshot(getCycleById(cycleId));
Expand All @@ -29,41 +33,82 @@ export const SidebarChart: FC<ProgressChartProps> = observer((props) => {
const totalEstimatePoints = cycleDetails?.total_estimate_points || 0;
const totalIssues = cycleDetails?.total_issues || 0;
const estimateType = getEstimateTypeByCycleId(cycleId);

const isCurrentProjectEstimateEnabled = projectId && areEstimateEnabledByProjectId(projectId) ? true : false;
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

Simplify unnecessary boolean ternary operator

The ternary operator in line 36 is unnecessary because the expression already evaluates to a boolean value. You can simplify the code by removing the ternary operator.

Apply this diff to fix:

- const isCurrentProjectEstimateEnabled = projectId && areEstimateEnabledByProjectId(projectId) ? true : false;
+ const isCurrentProjectEstimateEnabled = Boolean(projectId && areEstimateEnabledByProjectId(projectId));
📝 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
const isCurrentProjectEstimateEnabled = projectId && areEstimateEnabledByProjectId(projectId) ? true : false;
const isCurrentProjectEstimateEnabled = Boolean(projectId && areEstimateEnabledByProjectId(projectId));
🧰 Tools
🪛 Biome

[error] 36-36: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

const estimateDetails =
isCurrentProjectEstimateEnabled && currentActiveEstimateId && estimateById(currentActiveEstimateId);
const isCurrentEstimateTypeIsPoints = estimateDetails && estimateDetails?.type === EEstimateSystem.POINTS;
const chartDistributionData =
estimateType === "points" ? cycleDetails?.estimate_distribution : cycleDetails?.distribution || undefined;

const completionChartDistributionData = chartDistributionData?.completion_chart || undefined;

if (!workspaceSlug || !projectId || !cycleId) return null;

const isArchived = !!cycleDetails?.archived_at;

// handlers
const onChange = async (value: TCycleEstimateType) => {
setEstimateType(cycleId, value);
if (!workspaceSlug || !projectId || !cycleId) return;
try {
if (isArchived) {
await fetchArchivedCycleDetails(workspaceSlug, projectId, cycleId);
} else {
await fetchCycleDetails(workspaceSlug, projectId, cycleId);
}
} catch (err) {
console.error(err);
setEstimateType(cycleId, estimateType);
}
};
return (
<div>
<div className="relative flex items-center gap-2">
<div className="flex items-center justify-center gap-1 text-xs">
<span className="h-2.5 w-2.5 rounded-full bg-[#A9BBD0]" />
<span>Ideal</span>
<>
{isCurrentEstimateTypeIsPoints && (
<div className="relative flex items-center justify-between gap-2 pt-4">
<CustomSelect
value={estimateType}
label={<span>{cycleEstimateOptions.find((v) => v.value === estimateType)?.label ?? "None"}</span>}
onChange={onChange}
maxHeight="lg"
buttonClassName="border-none rounded text-sm font-medium capitalize"
>
{cycleEstimateOptions.map((item) => (
<CustomSelect.Option key={item.value} value={item.value} className="capitalize">
{item.label}
</CustomSelect.Option>
))}
</CustomSelect>
</div>
<div className="flex items-center justify-center gap-1 text-xs">
<span className="h-2.5 w-2.5 rounded-full bg-[#4C8FFF]" />
<span>Current</span>
)}
<div className="py-4">
<div>
<div className="relative flex items-center gap-2">
<div className="flex items-center justify-center gap-1 text-xs">
<span className="h-2.5 w-2.5 rounded-full bg-[#A9BBD0]" />
<span>Ideal</span>
</div>
<div className="flex items-center justify-center gap-1 text-xs">
<span className="h-2.5 w-2.5 rounded-full bg-[#4C8FFF]" />
<span>Current</span>
</div>
</div>
{cycleStartDate && cycleEndDate && completionChartDistributionData ? (
<Fragment>
<ProgressChart
distribution={completionChartDistributionData}
startDate={cycleStartDate}
endDate={cycleEndDate}
totalIssues={estimateType === "points" ? totalEstimatePoints : totalIssues}
plotTitle={estimateType === "points" ? "points" : "issues"}
/>
</Fragment>
) : (
<Loader className="w-full h-[160px] mt-4">
<Loader.Item width="100%" height="100%" />
</Loader>
)}
</div>
</div>
{cycleStartDate && cycleEndDate && completionChartDistributionData ? (
<Fragment>
<ProgressChart
distribution={completionChartDistributionData}
startDate={cycleStartDate}
endDate={cycleEndDate}
totalIssues={estimateType === "points" ? totalEstimatePoints : totalIssues}
plotTitle={estimateType === "points" ? "points" : "issues"}
/>
</Fragment>
) : (
<Loader className="w-full h-[160px] mt-4">
<Loader.Item width="100%" height="100%" />
</Loader>
)}
</div>
</>
);
});
76 changes: 9 additions & 67 deletions web/core/components/cycles/analytics-sidebar/issue-progress.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import { observer } from "mobx-react";
import { useSearchParams } from "next/navigation";
import { ChevronUp, ChevronDown } from "lucide-react";
import { Disclosure, Transition } from "@headlessui/react";
import { ICycle, IIssueFilterOptions, TCycleEstimateType, TCyclePlotType, TProgressSnapshot } from "@plane/types";
import { CustomSelect } from "@plane/ui";
import { ICycle, IIssueFilterOptions, TCyclePlotType, TProgressSnapshot } from "@plane/types";
// components
import { CycleProgressStats } from "@/components/cycles";
// constants
Expand All @@ -26,6 +25,11 @@ type TCycleAnalyticsProgress = {
cycleId: string;
};

export const cycleEstimateOptions: options[] = [
{ value: "issues", label: "issues" },
{ value: "points", label: "points" },
];

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 renaming options type to Options for consistency

The type options is currently in lowercase. In TypeScript, it's conventional to use PascalCase for type and interface names. Renaming options to Options will improve code readability and maintain consistency throughout the codebase.

export const validateCycleSnapshot = (cycleDetails: ICycle | null): ICycle | null => {
if (!cycleDetails || cycleDetails === null) return cycleDetails;

Expand All @@ -49,24 +53,14 @@ export const cycleChartOptions: options[] = [
{ value: "burndown", label: "Burn-down" },
{ value: "burnup", label: "Burn-up" },
];
export const cycleEstimateOptions: options[] = [
{ value: "issues", label: "issues" },
{ value: "points", label: "points" },
];

export const CycleAnalyticsProgress: FC<TCycleAnalyticsProgress> = observer((props) => {
// props
const { workspaceSlug, projectId, cycleId } = props;
// router
const searchParams = useSearchParams();
const peekCycle = searchParams.get("peekCycle") || undefined;
const {
getPlotTypeByCycleId,
getEstimateTypeByCycleId,
getCycleById,
fetchCycleDetails,
fetchArchivedCycleDetails,
setEstimateType,
} = useCycle();
const { getPlotTypeByCycleId, getEstimateTypeByCycleId, getCycleById } = useCycle();
const {
issuesFilter: { issueFilters, updateFilters },
} = useIssues(EIssuesStoreType.CYCLE);
Expand All @@ -76,21 +70,9 @@ export const CycleAnalyticsProgress: FC<TCycleAnalyticsProgress> = observer((pro
const plotType: TCyclePlotType = getPlotTypeByCycleId(cycleId);
const estimateType = getEstimateTypeByCycleId(cycleId);

const completedIssues = cycleDetails?.completed_issues || 0;
const totalIssues = cycleDetails?.total_issues || 0;
const completedEstimatePoints = cycleDetails?.completed_estimate_points || 0;
const totalEstimatePoints = cycleDetails?.total_estimate_points || 0;

const progressHeaderPercentage = cycleDetails
? estimateType === "points"
? completedEstimatePoints != 0 && totalEstimatePoints != 0
? Math.round((completedEstimatePoints / totalEstimatePoints) * 100)
: 0
: completedIssues != 0 && completedIssues != 0
? Math.round((completedIssues / totalIssues) * 100)
: 0
: 0;

const chartDistributionData =
estimateType === "points" ? cycleDetails?.estimate_distribution : cycleDetails?.distribution || undefined;

Expand All @@ -115,23 +97,6 @@ export const CycleAnalyticsProgress: FC<TCycleAnalyticsProgress> = observer((pro
const isCycleStartDateValid = cycleStartDate && cycleStartDate <= new Date();
const isCycleEndDateValid = cycleStartDate && cycleEndDate && cycleEndDate >= cycleStartDate;
const isCycleDateValid = isCycleStartDateValid && isCycleEndDateValid;
const isArchived = !!cycleDetails?.archived_at;

// handlers
const onChange = async (value: TCycleEstimateType) => {
setEstimateType(cycleId, value);
if (!workspaceSlug || !projectId || !cycleId) return;
try {
if (isArchived) {
await fetchArchivedCycleDetails(workspaceSlug, projectId, cycleId);
} else {
await fetchCycleDetails(workspaceSlug, projectId, cycleId);
}
} catch (err) {
console.error(err);
setEstimateType(cycleId, estimateType);
}
};

const handleFiltersUpdate = useCallback(
(key: keyof IIssueFilterOptions, value: string | string[]) => {
Expand Down Expand Up @@ -192,30 +157,7 @@ export const CycleAnalyticsProgress: FC<TCycleAnalyticsProgress> = observer((pro

<Transition show={open}>
<Disclosure.Panel className="flex flex-col">
<div className="relative flex items-center justify-between gap-2 pt-4">
<CustomSelect
value={estimateType}
label={<span>{cycleEstimateOptions.find((v) => v.value === estimateType)?.label ?? "None"}</span>}
onChange={onChange}
maxHeight="lg"
buttonClassName="border-none rounded text-sm font-medium"
>
{cycleEstimateOptions.map((item) => (
<CustomSelect.Option key={item.value} value={item.value}>
{item.label}
</CustomSelect.Option>
))}
</CustomSelect>
<div className="flex items-center justify-center gap-2">
<div className="flex items-center gap-1 text-xs">
<span className="text-custom-text-300">Done</span>
<span className="font-semibold text-custom-text-400">{progressHeaderPercentage}%</span>
</div>
</div>
</div>
<div className="py-4">
<SidebarChartRoot workspaceSlug={workspaceSlug} projectId={projectId} cycleId={cycleId} />
</div>
<SidebarChartRoot workspaceSlug={workspaceSlug} projectId={projectId} cycleId={cycleId} />
{/* progress detailed view */}
{chartDistributionData && (
<div className="w-full border-t border-custom-border-200 py-4">
Expand Down
2 changes: 1 addition & 1 deletion web/core/components/cycles/list/cycle-list-item-action.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export const CycleListItemAction: FC<Props> = observer((props) => {
<>
<button
onClick={openCycleOverview}
className={`z-[1] flex text-custom-primary-200 text-xs gap-1 flex-shrink-0 ${isMobile || isActive ? "flex" : "hidden group-hover:flex"}`}
className={`z-[1] flex text-custom-primary-200 text-xs gap-1 flex-shrink-0 ${isMobile || (isActive && !searchParams.has("peekCycle")) ? "flex" : "hidden group-hover:flex"}`}
>
<Eye className="h-4 w-4 my-auto text-custom-primary-200" />
<span>More details</span>
Expand Down
Loading