-
Notifications
You must be signed in to change notification settings - Fork 208
fix: added responsive-table class in few tables for mobile responsivness #2757
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
base: main
Are you sure you want to change the base?
Conversation
Console (appwrite/console)Project ID: Tip Build commands execute in runtime containers during deployment |
WalkthroughFour Svelte components are updated to add responsive table styling through a wrapping div (.responsive-table) with corresponding CSS for overflow handling and scrollbar styling. Changes include: recordTable column width increased from 100 to 200, conditional CAA row rendering based on $regionalConsoleVariables._APP_DOMAIN_TARGET_CAA, activity.svelte user fallback logic updated to use nullish coalescing operator, and sessions page enhanced with current-session badge and fallback for missing country data. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks✅ Passed checks (3 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/lib/components/domains/cnameTable.svelte (1)
75-83: Extract duplicated CSS to a shared stylesheet.The
.responsive-tablestyles are identical across all 4 files in this PR. Extract this to a shared stylesheet insrc/lib/styles/to follow DRY principles and ensure consistent maintenance.🔎 Suggested approach
Create a shared stylesheet (e.g.,
src/lib/styles/tables.css) and import it using@importor include it in your global styles:/* src/lib/styles/tables.css */ .responsive-table { overflow-x: auto; width: 100%; scrollbar-width: thin; position: relative; } /* Add webkit scrollbar styles for Chrome/Safari */ .responsive-table::-webkit-scrollbar { height: 6px; } .responsive-table::-webkit-scrollbar-thumb { background-color: rgba(0, 0, 0, 0.2); border-radius: 3px; }Then remove the
<style>blocks from all component files.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/lib/components/domains/cnameTable.sveltesrc/lib/components/domains/recordTable.sveltesrc/lib/layout/activity.sveltesrc/routes/(console)/account/sessions/+page.svelte
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/account/sessions/+page.sveltesrc/lib/layout/activity.sveltesrc/lib/components/domains/cnameTable.sveltesrc/lib/components/domains/recordTable.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/account/sessions/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/account/sessions/+page.sveltesrc/lib/layout/activity.sveltesrc/lib/components/domains/cnameTable.sveltesrc/lib/components/domains/recordTable.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/account/sessions/+page.sveltesrc/lib/layout/activity.sveltesrc/lib/components/domains/cnameTable.sveltesrc/lib/components/domains/recordTable.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/account/sessions/+page.svelte
src/lib/components/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure
Files:
src/lib/components/domains/cnameTable.sveltesrc/lib/components/domains/recordTable.svelte
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/account/sessions/+page.svelte
🔇 Additional comments (2)
src/routes/(console)/account/sessions/+page.svelte (1)
143-148: LGTM: Current session indicator improves user experience.The addition of the "current session" badge helps users quickly identify their active session in the table.
src/lib/layout/activity.svelte (1)
77-77: LGTM: Nullish coalescing operator is the correct choice.Using
??instead of||properly handles the case wherelog.userNameis an empty string (which is falsy but should still be displayed) versusnullorundefined.
| <div class="responsive-table"> | ||
| <Table.Root | ||
| columns={[ | ||
| { id: 'type', width: { min: 150 } }, | ||
| { id: 'name', width: { min: 80 } }, | ||
| { id: 'value', width: { min: 100 } } | ||
| ]} | ||
| let:root> | ||
| <svelte:fragment slot="header" let:root> | ||
| <Table.Header.Cell column="type" {root}>Type</Table.Header.Cell> | ||
| <Table.Header.Cell column="name" {root}>Name</Table.Header.Cell> | ||
| <Table.Header.Cell column="value" {root}>Value</Table.Header.Cell> | ||
| </svelte:fragment> | ||
| <Table.Row.Base {root}> | ||
| <Table.Cell column="type" {root}>CNAME</Table.Cell> | ||
| <Table.Cell column="name" {root}>{subdomain}</Table.Cell> | ||
| <Table.Cell column="value" {root}> | ||
| <InteractiveText | ||
| variant="copy" | ||
| isVisible | ||
| text={$regionalConsoleVariables._APP_DOMAIN_TARGET_CNAME} /> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| </Table.Root> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: overflow: hidden prevents scrolling on mobile devices.
The responsive wrapper uses overflow: hidden, which will clip table content when it exceeds the container width on mobile devices. This defeats the purpose of adding responsive table support, as users won't be able to access clipped content.
🔎 Proposed fix
Update the CSS to enable horizontal scrolling:
.responsive-table {
- overflow: hidden;
+ overflow-x: auto;
width: 100%;
scrollbar-width: thin;
position: relative;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/components/domains/cnameTable.svelte around lines 38 to 62 the
responsive wrapper currently uses overflow: hidden which clips table content on
narrow viewports; change the CSS for the .responsive-table wrapper to allow
horizontal scrolling instead (e.g., remove overflow:hidden and use overflow-x:
auto; overflow-y: visible; and add -webkit-overflow-scrolling: touch for smooth
scrolling), optionally add white-space: nowrap or ensure the inner table can
expand so users can scroll to access clipped columns on mobile.
| <div class="responsive-table"> | ||
| <Table.Root | ||
| columns={[ | ||
| { id: 'type', width: { min: 150 } }, | ||
| { id: 'name', width: { min: 80 } }, | ||
| { id: 'value', width: { min: 200 } } | ||
| ]} | ||
| let:root> | ||
| <svelte:fragment slot="header" let:root> | ||
| <Table.Header.Cell column="type" {root}>Type</Table.Header.Cell> | ||
| <Table.Header.Cell column="name" {root}>Name</Table.Header.Cell> | ||
| <Table.Header.Cell column="value" {root}>Value</Table.Header.Cell> | ||
| </svelte:fragment> | ||
|
|
||
| <Table.Row.Base {root}> | ||
| <Table.Cell column="type" {root}> | ||
| <Layout.Stack gap="s" direction="row" alignItems="center"> | ||
| <span>CAA</span> | ||
| <Badge variant="secondary" size="xs" content="Recommended" /> | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
| <Table.Cell column="name" {root}>@</Table.Cell> | ||
| <Table.Cell column="type" {root}>{variant.toUpperCase()}</Table.Cell> | ||
| <Table.Cell column="name" {root}>{subdomain || '@'}</Table.Cell> | ||
| <Table.Cell column="value" {root}> | ||
| <InteractiveText variant="copy" isVisible text={caaText} /> | ||
| <InteractiveText variant="copy" isVisible text={setTarget()} /> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/if} | ||
| </Table.Root> | ||
|
|
||
| {#if $regionalConsoleVariables._APP_DOMAIN_TARGET_CAA} | ||
| <Table.Row.Base {root}> | ||
| <Table.Cell column="type" {root}> | ||
| <Layout.Stack direction="row" alignItems="center" gap="s"> | ||
| <span>CAA</span> | ||
| <Badge variant="secondary" size="xs" content="Recommended" /> | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
| <Table.Cell column="name" {root}>@</Table.Cell> | ||
| <Table.Cell column="value" {root}> | ||
| <InteractiveText variant="copy" isVisible text={caaText} /> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/if} | ||
| </Table.Root> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: overflow: hidden prevents scrolling on mobile devices.
The responsive wrapper uses overflow: hidden, which will clip table content when it exceeds the container width on mobile devices. Combined with the increased column width (line 109), this will make content inaccessible.
🔎 Proposed fix
Update the CSS (lines 180-185) to enable horizontal scrolling:
.responsive-table {
- overflow: hidden;
+ overflow-x: auto;
width: 100%;
scrollbar-width: thin;
position: relative;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/components/domains/recordTable.svelte around lines 104 to 141, the
responsive wrapper currently applies overflow: hidden which prevents horizontal
scrolling on narrow/mobile viewports; update the CSS block around lines 180-185
to allow horizontal scrolling instead (e.g., replace overflow: hidden with
overflow-x: auto or overflow: auto) and add touch-friendly scrolling (e.g.,
-webkit-overflow-scrolling: touch) so the table can be scrolled horizontally on
mobile without clipping content.
| columns={[ | ||
| { id: 'type', width: { min: 150 } }, | ||
| { id: 'name', width: { min: 80 } }, | ||
| { id: 'value', width: { min: 200 } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconsider increasing the column width for mobile responsiveness.
Increasing the value column's min width from 100 to 200 makes the table wider and harder to fit on mobile screens. This contradicts the PR's goal of improving mobile responsiveness. Consider keeping the original width or using a responsive approach where the width adapts based on viewport size.
💡 Alternative approaches
Option 1: Revert to the original width:
-{ id: 'value', width: { min: 200 } }
+{ id: 'value', width: { min: 100 } }Option 2: Use a flexible width without a fixed minimum:
-{ id: 'value', width: { min: 200 } }
+{ id: 'value', width: { min: 80 } }Option 3: Make long values wrap or truncate with ellipsis on mobile using CSS.
🤖 Prompt for AI Agents
In src/lib/components/domains/recordTable.svelte around line 109, the value
column's min width was increased from 100 to 200 which harms mobile
responsiveness; revert the min width back to 100 or replace the fixed min with a
responsive approach (use CSS media queries or viewport-based logic to set a
smaller min on narrow screens), or make cell content responsive by allowing
flexible width and applying wrapping/truncation with ellipsis for long values on
mobile so the table fits smaller viewports.
| <div class="responsive-table"> | ||
| <Table.Root {columns} let:root> | ||
| <svelte:fragment slot="header" let:root> | ||
| <Table.Header.Cell column="user" {root}>User</Table.Header.Cell> | ||
| <Table.Header.Cell column="event" {root}>Event</Table.Header.Cell> | ||
| <Table.Header.Cell column="location" {root}>Location</Table.Header.Cell> | ||
| <Table.Header.Cell column="ip" {root}>IP</Table.Header.Cell> | ||
| <Table.Header.Cell column="date" {root}>Date</Table.Header.Cell> | ||
| </svelte:fragment> | ||
| {#each logs.logs as log} | ||
| <Table.Row.Base {root}> | ||
| <Table.Cell column="user" {root}> | ||
| <Layout.Stack direction="row" alignItems="center"> | ||
| {#if log.userEmail} | ||
| {#if log.userName} | ||
| <AvatarInitials size="xs" name={log.userName} /> | ||
| <Trim>{log.userName}</Trim> | ||
| {:else} | ||
| <AvatarInitials size="xs" name={log.userEmail} /> | ||
| <Trim>{log.userEmail}</Trim> | ||
| {/if} | ||
| {:else} | ||
| <AvatarInitials size="xs" name={log.userEmail} /> | ||
| <Trim>{log.userEmail}</Trim> | ||
| <div class="avatar is-size-small"> | ||
| <span class="icon-anonymous" aria-hidden="true"></span> | ||
| </div> | ||
| <span class="text u-trim">{log.userName ?? 'Anonymous'}</span> | ||
| {/if} | ||
| {:else} | ||
| <div class="avatar is-size-small"> | ||
| <span class="icon-anonymous" aria-hidden="true"></span> | ||
| </div> | ||
| <span class="text u-trim">{log.userName ?? 'Anonymous'}</span> | ||
| {/if} | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
|
|
||
| <Table.Cell column="event" {root}> | ||
| {log.event} | ||
| </Table.Cell> | ||
| <Table.Cell column="event" {root}> | ||
| {log.event} | ||
| </Table.Cell> | ||
|
|
||
| <Table.Cell column="location" {root}> | ||
| {#if log.countryCode !== '--'} | ||
| {log.countryName} | ||
| {:else} | ||
| Unknown | ||
| {/if} | ||
| </Table.Cell> | ||
| <Table.Cell column="location" {root}> | ||
| {#if log.countryCode !== '--'} | ||
| {log.countryName} | ||
| {:else} | ||
| Unknown | ||
| {/if} | ||
| </Table.Cell> | ||
|
|
||
| <Table.Cell column="ip" {root}> | ||
| <InteractiveText variant="copy" text={log.ip} isVisible /> | ||
| </Table.Cell> | ||
| <Table.Cell column="ip" {root}> | ||
| <InteractiveText variant="copy" text={log.ip} isVisible /> | ||
| </Table.Cell> | ||
|
|
||
| <Table.Cell column="date" {root}> | ||
| <DualTimeView time={log.time} showDatetime /> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/each} | ||
| </Table.Root> | ||
| <Table.Cell column="date" {root}> | ||
| <DualTimeView time={log.time} showDatetime /> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/each} | ||
| </Table.Root> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: overflow: hidden prevents scrolling on mobile devices.
The responsive wrapper uses overflow: hidden, which will clip table content when it exceeds the container width. On mobile devices, users won't be able to scroll to view the IP or Date columns.
🔎 Proposed fix
Update the CSS (lines 133-138) to enable horizontal scrolling:
.responsive-table {
- overflow: hidden;
+ overflow-x: auto;
width: 100%;
scrollbar-width: thin;
position: relative;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/lib/layout/activity.svelte around lines 52 to 104, the responsive wrapper
currently uses overflow: hidden which prevents horizontal scrolling on mobile
and clips columns like IP/Date; change the CSS for the .responsive-table wrapper
(lines ~133-138) to allow horizontal scrolling by using overflow-x: auto (or
overflow: auto), add -webkit-overflow-scrolling: touch for smooth mobile
scrolling, and ensure the inner table has a min-width (e.g., min-width: 100% or
a wider fixed/min content width) so columns can overflow and be scrolled into
view.
| <div class="responsive-table"> | ||
| <Table.Root | ||
| let:root | ||
| columns={[ | ||
| { id: 'client', width: { min: 450 } }, | ||
| { id: 'location', width: { min: 200 } }, | ||
| { id: 'ip', width: { min: 330 } }, | ||
| { id: 'actions', width: 100 } | ||
| ]}> | ||
| <svelte:fragment slot="header" let:root> | ||
| <Table.Header.Cell column="client" {root}>Client</Table.Header.Cell> | ||
| <Table.Header.Cell column="location" {root}>Location</Table.Header.Cell> | ||
| <Table.Header.Cell column="ip" {root}>IP</Table.Header.Cell> | ||
| <Table.Header.Cell column="actions" {root} /> | ||
| </svelte:fragment> | ||
| {#each data.sessions.sessions as session} | ||
| {@const browser = getBrowser(session.clientCode)} | ||
| <Table.Row.Base {root}> | ||
| <Table.Cell column="client" {root}> | ||
| <Layout.Stack direction="row" alignItems="center"> | ||
| {#if session.clientName} | ||
| <div class="avatar is-size-small"> | ||
| {#if browser} | ||
| <img | ||
| height="20" | ||
| width="20" | ||
| src={getBrowser(session.clientCode).toString()} | ||
| alt={session.clientName} /> | ||
| {:else} | ||
| <Icon icon={IconGlobeAlt} size="s" /> | ||
| {/if} | ||
| </div> | ||
| <Trim> | ||
| {session.clientName} | ||
| {session.clientVersion} | ||
| on {session.osName} | ||
| {session.osVersion} | ||
| </Trim> | ||
| {:else} | ||
| <span class="avatar is-color-empty"></span> | ||
| <p class="text u-trim">Unknown</p> | ||
| {/if} | ||
| <div class="is-only-desktop"> | ||
| <Badge variant="secondary" content={session.provider} /> | ||
| </div> | ||
| <Trim> | ||
| {session.clientName} | ||
| {session.clientVersion} | ||
| on {session.osName} | ||
| {session.osVersion} | ||
| </Trim> | ||
| {#if session.current} | ||
| <Badge | ||
| type="success" | ||
| variant="secondary" | ||
| content="current session" /> | ||
| {/if} | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
| <Table.Cell column="location" {root}> | ||
| {#if session.countryCode !== '--'} | ||
| {session.countryName} | ||
| {:else} | ||
| <span class="avatar is-color-empty"></span> | ||
| <p class="text u-trim">Unknown</p> | ||
| {/if} | ||
| <div class="is-only-desktop"> | ||
| <Badge variant="secondary" content={session.provider} /> | ||
| </div> | ||
| {#if session.current} | ||
| <Badge type="success" variant="secondary" content="current session" /> | ||
| Unknown | ||
| {/if} | ||
| </Layout.Stack> | ||
| </Table.Cell> | ||
| <Table.Cell column="location" {root}> | ||
| {#if session.countryCode !== '--'} | ||
| {session.countryName} | ||
| {:else} | ||
| Unknown | ||
| {/if} | ||
| </Table.Cell> | ||
| <Table.Cell column="ip" {root}> | ||
| <InteractiveText variant="copy" text={session.ip} isVisible /> | ||
| </Table.Cell> | ||
| <Table.Cell column="actions" {root}> | ||
| <Button size="xs" secondary on:click={() => logout(session)}>Sign out</Button> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/each} | ||
| </Table.Root> | ||
| </Table.Cell> | ||
| <Table.Cell column="ip" {root}> | ||
| <InteractiveText variant="copy" text={session.ip} isVisible /> | ||
| </Table.Cell> | ||
| <Table.Cell column="actions" {root}> | ||
| <Button size="xs" secondary on:click={() => logout(session)} | ||
| >Sign out</Button> | ||
| </Table.Cell> | ||
| </Table.Row.Base> | ||
| {/each} | ||
| </Table.Root> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: overflow: hidden prevents scrolling on mobile devices.
The responsive wrapper uses overflow: hidden, which will clip table content when it exceeds the container width on mobile screens. Users won't be able to scroll horizontally to view the IP address or Sign out button.
🔎 Proposed fix
Update the CSS (lines 172-177) to enable horizontal scrolling:
.responsive-table {
- overflow: hidden;
+ overflow-x: auto;
width: 100%;
scrollbar-width: thin;
position: relative;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/routes/(console)/account/sessions/+page.svelte around lines 98-168, the
responsive wrapper currently uses overflow: hidden which prevents horizontal
scrolling on small screens; change the wrapper CSS to allow horizontal scroll by
replacing overflow: hidden with overflow-x: auto (or overflow: auto) and add
-webkit-overflow-scrolling: touch for smooth scrolling on iOS, ensure the
responsive-table keeps width: 100% (or min-width on the inner table) so content
can be scrolled horizontally rather than clipped.

What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.