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

Fixed scroll issue in shifting in facilities. #9268

Closed
wants to merge 12 commits into from
+57 −16
Closed

Fixed scroll issue in shifting in facilities. #9268

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
fcaf7f5
Fixed scroll issue in shifting in facilities. Closes #7487
Jeffrin2005 Dec 2, 2024
9a6a1d9
Changes Updated Closes #9268
Jeffrin2005 Dec 4, 2024
4dd6525
Merge branch 'develop' into scroll-bar
Jeffrin2005 Dec 4, 2024
60ba91e
Merge branch 'develop' into scroll-bar
Jeffrin2005 Dec 4, 2024
25bed76
UPD
Jeffrin2005 Dec 4, 2024
e22701b
UPD
Jeffrin2005 Dec 4, 2024
e949a5f
Resolved
Jeffrin2005 Dec 4, 2024
02ccceb
LTS
Jeffrin2005 Dec 4, 2024
69d5519
Edited section
Jeffrin2005 Dec 4, 2024
c7ef408
Merge branch 'develop' into scroll-bar
Jeffrin2005 Dec 5, 2024
f555c30
Merge branch 'develop' into scroll-bar
Jeffrin2005 Dec 6, 2024
1cb7131
Merge branch 'develop' into scroll-bar
Jeffrin2005 Dec 10, 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
2 changes: 1 addition & 1 deletion package-lock.json
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

#9223 has solved issues with line endings on windows. Ensure proper line endings by using latest develop branch.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions src/CAREUI/display/ScrollableColumn.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// src/CAREUI/display/ScrollableColumn.tsx
import React from "react";

interface ScrollableColumnProps {
title: string;
children: React.ReactNode;
}

const ScrollableColumn: React.FC<ScrollableColumnProps> = ({
title,
children,
}) => {
return (
<div
style={{
height: "400px",
overflowY: "auto",
border: "1px solid #ccc",
margin: "10px",
}}
>
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 the component more flexible and accessible

The component could benefit from the following improvements:

  1. Allow customizable height through props
  2. Add ARIA attributes for better accessibility
  3. Support custom styling through className prop
 <div
+  role="region"
+  aria-label={title}
   style={{
-    height: "400px",
+    height: props.height || "400px",
     overflowY: "auto",
     border: "1px solid #ccc",
     margin: "10px",
+    ...props.style,
   }}
+  className={props.className}
 >

Update the props interface accordingly:

interface ScrollableColumnProps {
  title: string;
  children: React.ReactNode;
  height?: string;
  style?: React.CSSProperties;
  className?: string;
}

Copy link
Member

Choose a reason for hiding this comment

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

  1. Let's use shadcn's ScrollArea instead of custom building it https://ui.shadcn.com/docs/components/scroll-area

  2. Since this is a tailwindcss based project, we do not use css styles if tailwind classes can do it.

<h3>{title}</h3>
{children}
</div>
);
};

export default ScrollableColumn;
Empty file added src/CAREUI/interactive/temp.tsx
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

this file seems unnecessary

Empty file.
28 changes: 14 additions & 14 deletions src/components/Kanban/Board.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,14 @@ interface KanbanBoardProps<T extends { id: string }> {
}[];
itemRender: (item: T) => ReactNode;
}

export default function KanbanBoard<T extends { id: string }>(
props: KanbanBoardProps<T>,
) {
const board = useRef<HTMLDivElement>(null);

return (
<div className="h-[calc(100vh-114px)] md:h-[calc(100vh-50px)]">
<div className="flex flex-col items-end justify-between md:flex-row">
<div className="flex flex-col items-center justify-between md:flex-row">
<div>{props.title}</div>
<div className="flex items-center gap-2 py-2">
{[0, 1].map((button, i) => (
Expand All @@ -56,8 +55,15 @@ export default function KanbanBoard<T extends { id: string }>(
))}
</div>
</div>

<DragDropContext onDragEnd={props.onDragEnd}>
<div className="h-full overflow-scroll" ref={board}>
<div
className="h-full overflow-scroll"
ref={board}
style={{
overflow: "hidden",
}}
>
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

Resolve conflicting overflow styles

There's a potential issue with conflicting overflow styles:

  • Line 61 sets overflow-scroll
  • Line 64 overrides it with overflow: hidden

This might affect the intended scrolling behavior.

 <div
-  className="h-full overflow-scroll"
+  className="h-full overflow-x-auto overflow-y-hidden"
   ref={board}
-  style={{
-    overflow: "hidden",
-  }}
 >
📝 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
<div
className="h-full overflow-scroll"
ref={board}
style={{
overflow: "hidden",
}}
>
<div
className="h-full overflow-x-auto overflow-y-hidden"
ref={board}
>

<div className="flex items-stretch px-0 pb-2">
{props.sections.map((section, i) => (
<KanbanSection<T>
Expand All @@ -73,7 +79,6 @@ export default function KanbanBoard<T extends { id: string }>(
</div>
);
}

export function KanbanSection<T extends { id: string }>(
props: Omit<KanbanBoardProps<T>, "sections" | "onDragEnd"> & {
section: KanbanBoardProps<T>["sections"][number];
Expand All @@ -92,8 +97,6 @@ export function KanbanSection<T extends { id: string }>(
const defaultLimit = 14;
const { t } = useTranslation();

// should be replaced with useInfiniteQuery when we move over to react query

const fetchNextPage = async (refresh: boolean = false) => {
if (!refresh && (fetchingNextPage || !hasMore)) return;
if (refresh) setPages([]);
Expand Down Expand Up @@ -121,7 +124,6 @@ export function KanbanSection<T extends { id: string }>(
const sectionElementHeight =
sectionRef.current?.getBoundingClientRect().height;
const scrolled = props.boardRef.current?.scrollTop;
// if user has scrolled 3/4th of the current items
if (
scrolled &&
sectionElementHeight &&
Expand All @@ -130,7 +132,6 @@ export function KanbanSection<T extends { id: string }>(
fetchNextPage();
}
};

props.boardRef.current?.addEventListener("scroll", onBoardReachEnd);
return () =>
props.boardRef.current?.removeEventListener("scroll", onBoardReachEnd);
Expand All @@ -139,15 +140,12 @@ export function KanbanSection<T extends { id: string }>(
useEffect(() => {
fetchNextPage(true);
}, [props.section]);

return (
<Droppable droppableId={section.id}>
{(provided) => (
<div
ref={provided.innerRef}
className={
"relative mr-2 w-[300px] shrink-0 rounded-xl bg-secondary-200"
}
className="relative mr-2 w-[300px] shrink-0 rounded-xl bg-secondary-200"
>
<div className="sticky top-0 rounded-xl bg-secondary-200 pt-2">
<div className="mx-2 flex items-center justify-between rounded-lg border border-secondary-300 bg-white p-4">
Expand All @@ -159,7 +157,10 @@ export function KanbanSection<T extends { id: string }>(
</div>
</div>
</div>
<div ref={sectionRef}>
<div
ref={sectionRef}
className="h-[calc(100vh-180px)] overflow-y-auto"
>
{!fetchingNextPage && totalCount === 0 && (
<div className="flex items-center justify-center py-10 text-secondary-500">
{t("no_results_found")}
Expand Down Expand Up @@ -190,5 +191,4 @@ export function KanbanSection<T extends { id: string }>(
</Droppable>
);
}

export type KanbanBoardType = typeof KanbanBoard;
Loading