Fixed scroll issue in shifting in facilities. #9268
Conversation
WalkthroughA new React functional component named Changes
Assessment against linked issues
Possibly related issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/CAREUI/display/ScrollableColumn.tsx (1)
4-7: Add JSDoc documentation for the interfaceConsider adding TypeScript documentation to improve code maintainability and IDE support.
+/** + * Props for the ScrollableColumn component + * @property {string} title - The title displayed at the top of the column + * @property {React.ReactNode} children - The content to be displayed in the scrollable area + */ interface ScrollableColumnProps { title: string; children: React.ReactNode; }src/components/Kanban/Board.tsx (2)
160-163: Consider using ResizeObserver for dynamic height calculationThe hard-coded height calculation
h-[calc(100vh-180px)]might not work well across different screen sizes and layouts. Consider using ResizeObserver for dynamic height calculation.const sectionRef = useRef<HTMLDivElement>(null); const [sectionHeight, setSectionHeight] = useState("calc(100vh - 180px)"); useEffect(() => { const resizeObserver = new ResizeObserver((entries) => { const containerHeight = entries[0].contentRect.height; setSectionHeight(`${containerHeight}px`); }); if (sectionRef.current) { resizeObserver.observe(sectionRef.current); } return () => resizeObserver.disconnect(); }, []);
148-148: Consider using CSS Grid for responsive layoutThe fixed width
w-[300px]might not be optimal for all screen sizes. Consider using CSS Grid for a more responsive layout.-className="relative mr-2 w-[300px] shrink-0 rounded-xl bg-secondary-200" +className="relative mr-2 min-w-[300px] max-w-[400px] flex-1 rounded-xl bg-secondary-200"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
src/CAREUI/display/ScrollableColumn.tsx(1 hunks)src/components/Kanban/Board.tsx(4 hunks)
🔇 Additional comments (1)
src/components/Kanban/Board.tsx (1)
39-39: Verify the impact of alignment change
The change from items-end to items-center in the flex container might affect the visual alignment of elements. Please ensure this change doesn't create any UI inconsistencies.
✅ Verification successful
Alignment change is consistent with design patterns
The change from items-end to items-center in the flex container aligns with the codebase's design patterns. The search results show that items-center is widely used throughout the application for vertically centering content in flex containers, particularly in responsive layouts with md:flex-row breakpoints. This change maintains consistency with the established UI patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other components that might be affected by this alignment change
rg "items-end|items-center" --type tsx
Length of output: 68
Script:
#!/bin/bash
# Search for Tailwind alignment classes in all files, not just tsx
rg "items-end|items-center" -A 2 -B 2
Length of output: 70445
| <div | ||
| style={{ | ||
| height: "400px", | ||
| overflowY: "auto", | ||
| border: "1px solid #ccc", | ||
| margin: "10px", | ||
| }} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make the component more flexible and accessible
The component could benefit from the following improvements:
- Allow customizable height through props
- Add ARIA attributes for better accessibility
- 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;
}
src/components/Kanban/Board.tsx
Outdated
| <div | ||
| className="h-full overflow-scroll" | ||
| ref={board} | ||
| style={{ | ||
| overflow: "hidden", | ||
| }} | ||
| > |
There was a problem hiding this comment.
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.
| <div | |
| className="h-full overflow-scroll" | |
| ref={board} | |
| style={{ | |
| overflow: "hidden", | |
| }} | |
| > | |
| <div | |
| className="h-full overflow-x-auto overflow-y-hidden" | |
| ref={board} | |
| > |
Jeffrin2005
changed the title
| 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", | ||
| }} | ||
| > |
There was a problem hiding this comment.
-
Let's use shadcn's ScrollArea instead of custom building it https://ui.shadcn.com/docs/components/scroll-area
-
Since this is a tailwindcss based project, we do not use css styles if tailwind classes can do it.
src/CAREUI/interactive/temp.tsx
Outdated
There was a problem hiding this comment.
this file seems unnecessary
package-lock.json
Outdated
There was a problem hiding this comment.
#9223 has solved issues with line endings on windows. Ensure proper line endings by using latest develop branch.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/ui/scroll-area.tsx (2)
6-6: Add JSDoc documentation for better maintainabilityConsider adding JSDoc documentation to describe the component's purpose, props, and usage examples.
+/** + * A customizable scroll area component that provides a consistent scrolling experience + * across different browsers and platforms. + * + * @component + * @example + * ```tsx + * <ScrollArea className="h-[200px]"> + * <div>Scrollable content here</div> + * </ScrollArea> + * ``` + */ const ScrollArea = React.forwardRef<
24-44: Consider improving type safety for orientation propThe implementation looks great with proper touch handling and dark mode support. Consider enhancing type safety for the orientation prop.
+type ScrollBarOrientation = "vertical" | "horizontal"; + const ScrollBar = React.forwardRef< React.ElementRef<typeof ScrollAreaPrimitive.ScrollAreaScrollbar>, - React.ComponentPropsWithoutRef<typeof ScrollAreaPrimitive.ScrollAreaScrollbar> + React.ComponentPropsWithoutRef<typeof ScrollAreaPrimitive.ScrollAreaScrollbar> & { + orientation?: ScrollBarOrientation; + } >(({ className, orientation = "vertical", ...props }, ref) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.gitattributes(1 hunks)src/components/Kanban/Board.tsx(4 hunks)src/components/ui/scroll-area.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitattributes
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Kanban/Board.tsx
🔇 Additional comments (4)
src/components/ui/scroll-area.tsx (4)
1-4: LGTM! Good choice of dependencies
The use of Radix UI primitives is excellent for ensuring accessibility and consistent behavior across browsers. The imports are well-organized and include all necessary dependencies.
6-22: LGTM! Well-structured ScrollArea implementation
The component is well-implemented with proper TypeScript types, ref forwarding, and className handling. The use of Radix UI primitives ensures good accessibility and consistent behavior.
46-46: LGTM! Clean exports
The named exports are well-organized and support tree-shaking.
1-46: Verify integration with Kanban board components
The ScrollArea component looks well-implemented for solving the scroll issues. Let's verify its integration with the Kanban board components.
|
👋 Hi, @Jeffrin2005, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
github-actions
bot
added
merge conflict
|
👋 Hi, @Jeffrin2005, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
👋 Hi, @Jeffrin2005, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
Fixes Scroll issue in shifting in facilities. #7487
Added scroll bars for each box in the Kanban board.
scroll_bar1.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
ScrollableColumncomponent for enhanced vertical scrolling and layout management.Improvements
KanbanBoardandKanbanSectioncomponents for better user experience.Dependency Updates
@radix-ui/react-scroll-areadependency to version 1.2.1 for improved functionality.