fix: improve layout and responsiveness across various components#2305
fix: improve layout and responsiveness across various components#2305myselfprincee wants to merge 11 commits intokarakeep-app:mainfrom
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUI-only layout and spacing changes across dashboard components: responsive dialog/modal sizing, toolbar and tabs width/wrapping, list item/button positioning, editor/content min-widths, image rounding, and minor whitespace edits. No public APIs or runtime logic were modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (3)
apps/web/components/dashboard/preview/LinkContentSection.tsx (1)
178-178: Review the effectiveness ofmax-w-[80%]on a fixed-width element.The icon already has explicit
w-4(1rem width), so addingmax-w-[80%]has no effect. Themax-widthconstraint only applies when the content width would exceed it, butw-4explicitly sets the width. Consider removingmax-w-[80%]unless this was intended for a different layout scenario.🔎 Suggested simplification
- <IconComponent className="mr-2 h-4 w-4 max-w-[80%]" /> + <IconComponent className="mr-2 h-4 w-4" />apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx (2)
24-24: Verify the universal selector*:max-w-[95%]doesn't constrain nested components.The universal child selector (
*:max-w-[95%]) applies a max-width constraint to all direct children of DialogContent. This could unintentionally limit the width of internal components like buttons, headers, or editor toolbars that may need full width. Consider using more specific selectors or testing thoroughly to ensure nested components render as expected.
30-30: Consider whethermin-w-[100%]is necessary.The
min-w-[100%]class forces this container to take full width, but since it's inside a dialog content that already manages width constraints, this might be redundant. The container will naturally take full width within its parent unless other constraints prevent it.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsxapps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsxapps/web/components/dashboard/lists/AllListsView.tsxapps/web/components/dashboard/preview/BookmarkPreview.tsxapps/web/components/dashboard/preview/LinkContentSection.tsxapps/web/components/ui/dialog.tsxapps/web/components/ui/markdown/plugins/toolbar-plugin.tsxapps/web/components/ui/scroll-area.tsxapps/web/components/ui/tabs.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/web/components/ui/tabs.tsxapps/web/components/dashboard/lists/AllListsView.tsxapps/web/components/ui/scroll-area.tsxapps/web/components/dashboard/preview/LinkContentSection.tsxapps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsxapps/web/components/ui/markdown/plugins/toolbar-plugin.tsxapps/web/components/dashboard/preview/BookmarkPreview.tsxapps/web/components/ui/dialog.tsxapps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/web/components/ui/tabs.tsxapps/web/components/dashboard/lists/AllListsView.tsxapps/web/components/ui/scroll-area.tsxapps/web/components/dashboard/preview/LinkContentSection.tsxapps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsxapps/web/components/ui/markdown/plugins/toolbar-plugin.tsxapps/web/components/dashboard/preview/BookmarkPreview.tsxapps/web/components/ui/dialog.tsxapps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/web/components/ui/tabs.tsxapps/web/components/dashboard/lists/AllListsView.tsxapps/web/components/ui/scroll-area.tsxapps/web/components/dashboard/preview/LinkContentSection.tsxapps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsxapps/web/components/ui/markdown/plugins/toolbar-plugin.tsxapps/web/components/dashboard/preview/BookmarkPreview.tsxapps/web/components/ui/dialog.tsxapps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Use Tailwind CSS for styling in the web application
Use Next.js for building the main web application
Files:
apps/web/components/ui/tabs.tsxapps/web/components/dashboard/lists/AllListsView.tsxapps/web/components/ui/scroll-area.tsxapps/web/components/dashboard/preview/LinkContentSection.tsxapps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsxapps/web/components/ui/markdown/plugins/toolbar-plugin.tsxapps/web/components/dashboard/preview/BookmarkPreview.tsxapps/web/components/ui/dialog.tsxapps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
🧠 Learnings (1)
📚 Learning: 2025-11-27T16:02:47.440Z
Learnt from: CR
Repo: karakeep-app/karakeep PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-27T16:02:47.440Z
Learning: Applies to packages/web/components/ui/**/*.{ts,tsx} : Place shadcn/ui components in `packages/web/components/ui`
Applied to files:
apps/web/components/ui/scroll-area.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (8)
apps/web/components/ui/tabs.tsx (1)
46-46: LGTM!Adding
min-w-fullensures tab content consistently uses the full available width, which aligns well with the responsive layout improvements across the PR.apps/web/components/dashboard/preview/BookmarkPreview.tsx (2)
225-225: LGTM!The margin adjustments (
mx-2 my-2) provide appropriate spacing for the tabbed layout on narrow screens while maintaining the existing behavior on larger viewports.
235-235: LGTM!Adding
w-fullandjustify-centerensures consistent content alignment within the active tab panel on narrow screens.apps/web/components/ui/dialog.tsx (2)
42-42: LGTM!The responsive width adjustments improve dialog usability on smaller screens. The
max-w-[95%]default withsm:max-w-lgoverride provides better mobile experience while maintaining the existing desktop appearance.
79-79: LGTM!Adding vertical spacing (
gap-y-2.5) between stacked footer buttons on mobile, then removing it (sm:gap-y-0) on larger screens improves touch target spacing and overall mobile UX.apps/web/app/dashboard/@modal/(.)preview/[bookmarkId]/page.tsx (1)
37-37: LGTM!The responsive sizing approach maximizes screen usage on mobile devices (
h-[100%] max-h-full max-w-[100%]) while providing comfortable margins on larger screens (sm:max-h-[90%] sm:max-w-[90%]), which aligns well with the PR's responsive layout objectives.apps/web/components/ui/markdown/plugins/toolbar-plugin.tsx (2)
245-245: LGTM!Adding
min-w-fullandflex-wrapenables the toolbar to adapt to narrow screens by wrapping elements when necessary, which improves usability on mobile devices while maintaining the existing desktop layout.
264-288: LGTM!The restructured layout with responsive breakpoints (
min-w-full sm:min-w-fit,justify-between sm:justify-normal) and nested containers for the Save button and tooltip provides better organization and spacing across different screen sizes without altering the underlying functionality.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/components/dashboard/preview/BookmarkPreview.tsx">
<violation number="1" location="apps/web/components/dashboard/preview/BookmarkPreview.tsx:235">
P2: The `justify-center` class has no effect here because the element is not a flex or grid container. `flex-1` makes this a flex item, not a flex container. If centering content is intended, add `flex` along with `justify-center`, or use `flex items-center justify-center` for full centering.</violation>
</file>
<file name="apps/web/components/ui/scroll-area.tsx">
<violation number="1" location="apps/web/components/ui/scroll-area.tsx:17">
P1: Adding forced centering to all `ScrollArea` children may break existing layouts. This shared component is used for text/markdown content (e.g., `TextContentSection`, `LinkContentSection`) where content should be left-aligned and start at the top, not centered. Consider applying centering at specific usage sites that need it rather than modifying the shared component.</violation>
</file>
<file name="apps/web/components/dashboard/preview/LinkContentSection.tsx">
<violation number="1" location="apps/web/components/dashboard/preview/LinkContentSection.tsx:178">
P2: Inconsistent styling: `max-w-[80%]` is added to custom renderer icons but not to the default renderer icons (BookOpen, Camera, Archive, Video) in the same dropdown. If this constraint is needed for layout purposes, it should be applied consistently to all icons in the SelectGroup. Also, applying a percentage-based max-width to a fixed-size icon (`h-4 w-4`) is unusual and may not achieve the intended effect.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
@myselfprincee this is amazing. Thanks a lot for taking the time to find and fix those issuses. I'm a bit hesitant to merge the changes to the shadcn components as they'll have a much wider impact and the AI reviewers also suggest that it might be break stuff. So if you don't mind, I'll modify the PR a bit to selectively merge some of the fixes in PR. |
Yeah sure. I have no issues. Thanks a lot. |
myselfprincee
left a comment
myselfprincee
left a comment
There was a problem hiding this comment.
removed justify-center as it was marked unnecessary by cubic and i think it makes sense.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/dashboard/preview/BookmarkPreview.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/web/components/dashboard/preview/BookmarkPreview.tsx
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/web/components/dashboard/preview/BookmarkPreview.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/web/components/dashboard/preview/BookmarkPreview.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Use Tailwind CSS for styling in the web application
Use Next.js for building the main web application
Files:
apps/web/components/dashboard/preview/BookmarkPreview.tsx
🔇 Additional comments (1)
apps/web/components/dashboard/preview/BookmarkPreview.tsx (1)
235-235: LGTM! Width constraint ensures proper content display.Adding
w-fullto the TabsContent ensures the content area properly spans the full width of its container, which is essential for responsive layouts. This change aligns with standard Tailwind patterns for content areas.
Reverted DialogContent classname back to max-w-[80%]
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx (1)
30-30: Verify necessity ofmin-w-[100%]in light of past discussion.The
min-w-[100%]class added here is likely redundant since block-leveldivelements default to 100% width in normal document flow. Additionally, the past review discussion (lines 24-24) indicated that modal behavior on small screens requires a more generic solution, and this isolated change may not align with that broader approach.Please confirm whether this change is still desired or if it should be deferred until the generic modal solution is implemented.
🔎 Optional: Remove redundant min-width if unnecessary
- <div className="h-[80vh] min-w-[100%]"> + <div className="h-[80vh]">
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Use Tailwind CSS for styling in the web application
Use Next.js for building the main web application
Files:
apps/web/components/dashboard/bookmarks/BookmarkedTextEditor.tsx
…ithub.com/myselfprincee/karakeep into fix/alignment_issues_on_diff_screen_sizes
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/dashboard/preview/TextContentSection.tsx (1)
21-29: Styling change looks good, but consider updating deprecated Image props.The
rounded-mdaddition aligns well with the PR's layout improvements. However, thelayoutandobjectFitprops are deprecated in Next.js 13+. While not introduced by this PR, consider modernizing:🔎 Optional modernization for Next.js Image
<Image alt="banner" src={getAssetUrl(banner.id)} - className="rounded-md" - width={0} - height={0} - layout="fill" - objectFit="cover" + className="rounded-md object-cover" + fill />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsxapps/web/components/dashboard/preview/TextContentSection.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/web/components/dashboard/preview/TextContentSection.tsxapps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsx
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/web/components/dashboard/preview/TextContentSection.tsxapps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/web/components/dashboard/preview/TextContentSection.tsxapps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Use Tailwind CSS for styling in the web application
Use Next.js for building the main web application
Files:
apps/web/components/dashboard/preview/TextContentSection.tsxapps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsx
🔇 Additional comments (1)
apps/web/components/dashboard/bookmarks/BookmarkMarkdownComponent.tsx (1)
38-48: Verify horizontal centering is intentional for markdown content.The flex centering (
items-center) will horizontally center the markdown content within the container. For text-heavy bookmarks, this may cause the text block itself to appear centered rather than left-aligned, which could affect readability depending on howMarkdownReadonlyandMarkdownEditorhandle their internal widths.If the markdown components have constrained widths, this centering looks intentional and the change is fine. If they expand to fill available width, consider whether
items-startwould be more appropriate for text content.
Refactor BookmarkMarkdownComponent to simplify structure and improve readability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/components/ui/markdown/markdown-readonly.tsx
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/web/components/ui/markdown/markdown-readonly.tsx
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/web/components/ui/markdown/markdown-readonly.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/web/components/ui/markdown/markdown-readonly.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: Use Tailwind CSS for styling in the web application
Use Next.js for building the main web application
Files:
apps/web/components/ui/markdown/markdown-readonly.tsx
myselfprincee
left a comment
myselfprincee
left a comment
There was a problem hiding this comment.
This only seems a viable way to make the contents of the preview dialog without affecting other components and using conditional states.
here's an before and after
Before
After
2 participants
This Pull Request resolves multiple layout and alignment inconsistencies across the Karakeep web interface, with a focus on responsive behavior across screen sizes.
Below are visual comparisons demonstrating the improvements.
Before vs After (Component-level)
Before vs After
Before
After
Before vs After (Desktop Layout)
Before
After
Before vs After (on small screens)
Before
After
and more.
Summary of Improvements