Skip to content

Conversation

@AbhiVarde
Copy link
Contributor

@AbhiVarde AbhiVarde commented Nov 4, 2025

What does this PR do?

Problem

When viewing function execution details with only 6 rows per page in the table:

  • Execution details sheet doesn't scroll properly
  • Long log responses get cut off and are not fully visible
  • Main page has no scroll (only 6 rows), so sheet is constrained by viewport height
  • Users cannot access complete execution logs

This issue doesn't occur with larger page sizes (12, 24, 48, 96) because the main page becomes scrollable, allowing the sheet to expand fully.

Solution

Added a scrollable container wrapper around the sheet content with:

  • max-height: calc(100vh - 280px) to constrain height within viewport
  • overflow-y: auto to enable vertical scrolling
  • Ensures all execution details (Request, Response, logs) are accessible

The 280px offset is determined to ensure optimal visibility of the log content across different screen sizes.

Changes Made

File: sheet.svelte

  • Added limit value as a prop (passed from page.svelte > table.svelte > sheet.svelte)
  • Applied conditional class: <div class={limit === 6 ? 'log-content-scroll' : ''}>
  • Added .log-content-scroll wrapper for the sheet content to enable scrolling when limit === 6
  • Implemented CSS with max-height and overflow-y: auto for the scrollable behavior
  • No visual or functional impact for other page limits (e.g., 12, 24, 48, 96)

Test Plan

Local Testing Performed:

  1. ✅ Set executions per page to 6 rows
  2. ✅ Opened execution sheet with short log response - displays correctly
  3. ✅ Opened execution sheet with long log response - scrolls smoothly
  4. ✅ Tested with 12, 24, 48, 96 rows - no regression, still works perfectly
  5. ✅ Tested sheet opening/closing - smooth transitions

Cross-page Size Testing:

  • ✅ 6 rows - Sheet scrollable, full content visible
  • ✅ 12 rows - Works as before, no regression
  • ✅ 24 rows - Works as before, no regression
  • ✅ 48 rows - Works as before, no regression
  • ✅ 96 rows - Works as before, no regression

Screenshots/Video

Before Fix (6 rows per page):
https://github.com/user-attachments/assets/71fe7984-bd02-4bfc-9007-eaf4a9468321

After Fix (6 rows per page):
https://github.com/user-attachments/assets/2fc994b3-94c0-435d-82b2-13e98d173dd9

Related PRs and Issues

Fixes #2557

Have you read the Contributing Guidelines on issues?

Yes, I have read and followed the contributing guidelines.

Summary by CodeRabbit

  • New Features

    • Added per-page limit configuration for function executions table.
  • Improvements

    • Reorganized execution details layout for better readability.
    • Enhanced status display with scheduled timestamp information.
    • Improved request path presentation in execution details.
    • Optimized scrolling behavior for execution logs and content.

@appwrite
Copy link

appwrite bot commented Nov 4, 2025

Console

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Failed Failed Authorize Preview URL QR Code

Tip

Cursor pagination performs better than offset pagination when loading further pages.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

Walkthrough

The PR threads a new limit prop (number) through the executions view hierarchy: the page component passes it to the Table component, which forwards it to the Sheet component. The Sheet uses this limit value to conditionally apply CSS styling (log-content-scroll class when limit === 6) and to restructure its Details layout section, including reorganizing Status/Duration/Triggered-by display blocks, adjusting the Path/Created presentation with InteractiveText, and introducing new scrolling behavior constraints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12–18 minutes

  • sheet.svelte: Most attention needed here. Verify the layout restructuring (nested Layout.Stacks), Status/Duration/Triggered-by reorganization, Path/Created modifications with InteractiveText, and the conditional CSS class application (log-content-scroll when limit === 6). Ensure the DOM changes maintain proper alignment and visibility.
  • Prop threading pattern: Confirm the limit prop flows correctly through page → table → sheet with proper type consistency.

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: enabling scrolling in the execution sheet when page limit is small (6 rows).
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #2557: passes limit prop through component hierarchy, applies conditional scroll wrapper when limit===6, and enables vertical scrolling for long content.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the identified issue; prop additions, conditional styling, and scroll behavior are targeted and do not introduce unrelated modifications.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20e5930 and f48d877.

📒 Files selected for processing (3)
  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (1 hunks)
  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte (3 hunks)
  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T07:41:06.679Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2425
File: src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte:454-468
Timestamp: 2025-09-30T07:41:06.679Z
Learning: In `src/routes/(console)/project-[region]-[project]/databases/database-[database]/(suggestions)/empty.svelte`, the column suggestions API (console.suggestColumns) has a maximum limit of 7 columns returned, which aligns with the initial placeholder count of 7 in customColumns.

Applied to files:

  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte
  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte
  • src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte
🔇 Additional comments (7)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/+page.svelte (1)

48-48: LGTM! Clean prop threading.

The limit prop is correctly passed from the page data to the Table component, enabling the scroll fix downstream.

src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/table.svelte (2)

22-30: LGTM! Proper prop definition and typing.

The limit prop is correctly destructured from $props with appropriate TypeScript typing, following Svelte 5 patterns.


126-126: LGTM! Prop correctly forwarded.

The limit prop is properly passed to the Sheet component, completing the prop threading chain.

src/routes/(console)/project-[region]-[project]/functions/function-[function]/executions/sheet.svelte (4)

31-31: LGTM! Prop correctly exported.

The limit prop is properly defined and will be used to conditionally apply scroll behavior.


86-196: Verify the layout restructuring doesn't introduce regressions.

The Details section has been significantly reorganized with nested Layout.Stack components, restructured field groupings, and the addition of InteractiveText for the Path field. While these changes appear well-structured, they represent a broader refactor beyond just adding scroll functionality.

Please confirm:

  • The visual layout remains unchanged when limit !== 6 (per PR objectives: "No visual or functional impact for other page limits").
  • All execution details (Status, Duration, Triggered by, Path, Created, etc.) render correctly across different execution states (processing, waiting, scheduled, completed, failed).
  • The Status tooltip for scheduled executions displays properly within the new layout.
  • The InteractiveText component for Path provides the same copy/interaction behavior as before.

Testing these scenarios will ensure the restructuring is purely organizational and doesn't introduce unexpected visual or functional changes.


201-206: Verify the 280px offset is appropriate across viewport sizes and update if needed.

The max-height: calc(100vh - 280px) uses a hard-coded offset that:

  • Is not responsive—no media queries adjust it for smaller viewports
  • Differs from codebase patterns: other components use a consistent 48px offset (header height) or CSS variables like calc(100vh - var(--top) - 4rem)
  • Lacks documentation explaining what the 280px represents

On small screens (e.g., 768px height), this leaves ~488px scrollable. On landscape mobile (640px), it leaves ~360px.

Recommendation: Either align with the codebase pattern (48px or CSS variables) or add responsive adjustments via @media queries for smaller viewports to ensure adequate scrollable space.


85-85: The hard-coded limit === 6 check is currently the only viable approach given available options, but using <= would be a reasonable defensive improvement.

Based on verification, the limit options available to users are hardcoded as [6, 12, 24, 48, 96] in ./src/lib/components/limit.svelte. This means:

  • Only limit value 6 (the smallest) triggers the scroll class
  • No limit values < 6 currently exist, so the review's concern about "users select limits smaller than 6 (e.g., 3, 1)" is not currently valid
  • Using limit <= 6 would be functionally identical to limit === 6 today

However, changing to limit <= 6 remains a reasonable improvement for defensive programming: if smaller limit options are added in the future, the condition will automatically adapt without code changes. The suggestion improves maintainability without risk, even though the specific examples in the review are not currently applicable.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@AbhiVarde AbhiVarde closed this Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug: Execution log sheet content cutoff with small page sizes

1 participant