Skip to content

Conversation

hoophalab
Copy link
Contributor

@hoophalab hoophalab commented Sep 16, 2025

Description

This PR fixes #1305.

The progress bar on the search page appears to be wider than the content layout. This is because the progress bar is using position: absolute, which makes it "float" on the search page rather than being part of the document flow. Because of this, this sizes it based on the entire page's width instead of its parent container (antd Layout).

To fix this, I created a new container with position: relative and a height of 0. This new container will not take up any space on the page, but it will serve as a reference point for the absolutely positioned progress bar. By placing the progress bar inside this new container, it will now correctly use the layout's width instead of the page's.

If you have a better way to approach this (e.g. using position: fixed), please let me know.

Checklist

  • The PR satisfies the contribution guidelines.
  • This is a breaking change and that has been indicated in the PR title, OR this isn't a
    breaking change.
  • Necessary docs have been updated, OR no docs need to be updated.

Validation performed

The progress bar no longer overflows.

Summary by CodeRabbit

  • Style
    • Updated the progress bar on the Presto search page to use fixed positioning, keeping it visible as you scroll through results.
    • Prevents the indicator from shifting or being obscured on longer pages and across different screen sizes, improving continuity and feedback during operations.
    • Provides a more stable layout and smoother visual behaviour, reducing flicker and optimizing readability during active searches.

@hoophalab hoophalab requested a review from a team as a code owner September 16, 2025 19:54
Copy link
Contributor

coderabbitai bot commented Sep 16, 2025

Walkthrough

Updated CSS for the Presto progress bar: changed the .prestoProgress positioning from absolute to fixed. No other style rules were modified.

Changes

Cohort / File(s) Summary of Changes
Presto Progress Bar CSS
components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.module.css
Updated .prestoProgress position from absolute to fixed; other declarations unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning Issue #1305 requires the progress bar be constrained to its intended container so it does not overflow the window width; the PR objectives describe adding a position:relative container and placing the bar inside it, but the raw file summary only shows changing .prestoProgress from absolute to fixed with no evidence of the container addition. Because the implemented change does not match the described container-based fix and position:fixed is relative to the viewport, the current diff does not demonstrate that the coding requirement for [#1305] is met and may not resolve the overflow. The PR therefore fails to show compliance with the linked issue as currently implemented. Please implement the container-based solution described in the PR objectives by adding a positioned wrapper (e.g., position:relative and appropriate sizing) and placing the progress bar inside using position:absolute or width:100% so it is constrained to the layout, or provide a clear justification and tests/screenshots if position:fixed is intentionally used; update the diff and PR description accordingly so reviewers can verify the overflow is resolved for different viewport widths.
Title Check ❓ Inconclusive The title is concise and clearly states the intent to prevent the presto progress bar from overflowing its parent and references issue #1305, but the provided file-level summary shows only a CSS change switching .prestoProgress from position:absolute to position:fixed while the PR objectives describe adding a relative-positioned container; this mismatch makes it unclear whether the title accurately summarizes the implemented change. Because the code diff and the PR description disagree, there is insufficient information to confirm the title matches the actual changes. Please reconcile the description and diff so reviewers can verify the title's accuracy. Please align the PR metadata and code by either updating the code to include the new relative-positioned container described in the objectives or updating the title/description to reflect the actual CSS change (position: fixed); include the missing file changes or a brief justification for using position:fixed and add a before/after screenshot or link to a running example to verify the fix.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Out of Scope Changes Check ✅ Passed The changeset contains a single CSS modification (position: absolute -> fixed on .prestoProgress) which is directly related to progress bar positioning and therefore appears within the scope of issue #1305, and I do not see unrelated file edits or functional changes outside that objective. However, the implementation differs from the PR description (which claims a new container was added), so while the edit is in-scope it may be incomplete or incorrect relative to the intended fix. Reviewers should request the missing container change or a justification for the alternate approach.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7249bc3 and 4585194.

📒 Files selected for processing (1)
  • components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.module.css (1 hunks)
🔇 Additional comments (1)
components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.module.css (1)

2-2: Avoid position: fixed; it binds to the viewport, not the layout container, contradicting the PR objective.

This makes the bar span the full window and stay on screen while scrolling, likely misaligning with the antd Layout width. Use an in-flow relative container with an absolutely positioned bar instead.

Apply this change to the modified line:

-    position: fixed;
+    position: absolute;

Add these rules (outside this hunk) to match the PR description and prevent layout shift:

.prestoProgressContainer {
  position: relative;
  height: 0; /* ensures no extra layout space; overlay only */
}

.prestoProgress {
  top: 0;
  left: 0;
  right: 0;          /* ensure full width within container */
  pointer-events: none; /* avoid intercepting clicks while overlaying */
  /* z-index: 1;     // uncomment if it needs to overlay siblings */
}

Please confirm whether persistent-on-scroll behaviour is desired. If yes, we can revisit a fixed/sticky variant aligned to the layout gutters; otherwise, the container+absolute approach fulfils #1305 as stated.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9888c11 and 7249bc3.

📒 Files selected for processing (2)
  • components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.module.css (1 hunks)
  • components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

  • Prefer false == <expression> rather than !<expression>.

Files:

  • components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.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: package-image
🔇 Additional comments (1)
components/webui/client/src/pages/SearchPage/Presto/ProgressBar/index.module.css (1)

1-5: Scoped positioning context — fix looks good.

Adding a positioned container correctly constrains the absolutely positioned progress bar to the layout width. This addresses the overflow in #1305.

Please confirm this also resolves the issue on narrow viewports and high-DPI displays.

@hoophalab hoophalab changed the title fix(webui): Add a container to prevent the presto progress bar from overflowing its parent (fixes #1305). fix(webui): Prevent the presto progress bar from overflowing its parent (fixes #1305). Sep 23, 2025
@hoophalab hoophalab merged commit 82bdcce into y-scope:main Sep 23, 2025
19 checks passed
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.

Progress bar overflows window width

2 participants