Skip to content

Conversation

@urmauur
Copy link
Member

@urmauur urmauur commented Jul 30, 2025

Describe Your Changes

This pull request enhances the LeftPanel component in the web-app by integrating download state management and dynamically adjusting the layout based on download activity. The most important changes include adding the useDownloadStore hook, updating the layout to reflect download activity, and ensuring proper rendering of the DownloadManagement component.

Integration of download state management:

  • Added the useDownloadStore hook to the LeftPanel component to access downloads and localDownloadingModels state. (web-app/src/containers/LeftPanel.tsx, [1] [2]

Layout adjustments based on download activity:

  • Updated the height calculation of a container in the LeftPanel to dynamically adjust based on whether there are active downloads or local downloading models. (web-app/src/containers/LeftPanel.tsx, web-app/src/containers/LeftPanel.tsxL256-R266)

Rendering improvements:

Fixes Issues

Screen.Recording.2025-07-30.at.18.11.26.mp4

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

Important

Enhances LeftPanel by integrating download state management and adjusting layout based on download activity.

  • Behavior:
    • Integrates useDownloadStore hook in LeftPanel to manage downloads and localDownloadingModels state.
    • Adjusts container height in LeftPanel based on active downloads or local downloading models.
  • Rendering:
    • Ensures DownloadManagement component is properly rendered within LeftPanel.

This description was created by Ellipsis for f4fdaaf. You can customize this summary. It will automatically update as commits are pushed.

@urmauur urmauur self-assigned this Jul 30, 2025
@urmauur urmauur added this to the v0.6.6 milestone Jul 30, 2025
@urmauur urmauur moved this to Needs Review in Jan Jul 30, 2025
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to f4fdaaf in 1 minute and 58 seconds. Click for details.
  • Reviewed 47 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. web-app/src/containers/LeftPanel.tsx:45
  • Draft comment:
    Check the import path: 'DownloadManegement' may be a typo of 'DownloadManagement'. If unintentional, consider renaming for clarity.
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. web-app/src/containers/LeftPanel.tsx:172
  • Draft comment:
    Newly added useDownloadStore hook: Ensure it reliably returns defined values for downloads and localDownloadingModels to avoid any render issues.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. web-app/src/containers/LeftPanel.tsx:259
  • Draft comment:
    Conditional height calculation: Consider extracting the logic (switching between 'h-[calc(100%-200px)]' and 'h-[calc(100%-140px)]') into a variable for improved readability, and verify that removing the '!h-' override does not affect styling specificity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment has two parts: 1) Extracting logic to variable - this could improve readability but is a minor improvement 2) Questioning !h- override - but this seems speculative since we don't have full context of CSS specificity needs. The height calculation itself is fairly straightforward and readable as-is. The benefit of extracting it seems minimal. I could be undervaluing the readability benefits of extracting the logic. The !h- override question could be pointing to a real CSS specificity issue. While extraction could help slightly, the current inline logic is clear enough. The !h- specificity concern is too speculative without more context about CSS cascade needs. The comment's suggestions are not compelling enough to justify a code change. The current code is reasonably clear and functional as-is.
4. web-app/src/containers/LeftPanel.tsx:496
  • Draft comment:
    DownloadManagement component relocation: It is now rendered only once. Double-check its new position (within the bottom menu container) meets the intended UX for always-visible download progress.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the PR author to double-check the new position of the DownloadManagement component, which violates the rule against asking the author to confirm their intention or ensure behavior is intended. It does not provide a specific suggestion or point out a clear issue.

Workflow ID: wflow_Yu28VLNKhQ7yzk1W

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@github-actions
Copy link
Contributor

Barecheck - Code coverage report

Total: 38.63%

Your code coverage diff: 0.02% ▴

Uncovered files and lines
FileLines
web-app/src/containers/LeftPanel.tsx89-92, 104, 107-111, 114-125, 165, 181-183, 185-193, 203, 243-249, 251-252, 263, 268-274, 276-292, 294-295, 297, 302-330, 332-347, 349-350, 354-367, 369-378, 380-399, 401-423, 425-434, 438-448, 478, 488

@urmauur
Copy link
Member Author

urmauur commented Jul 30, 2025

Tested build 979 on Mac cc @Minh141120

Screen.Recording.2025-07-30.at.18.29.03.mp4

@Minh141120
Copy link
Member

LGTM on Windows!
image

@urmauur urmauur merged commit 1e7e572 into release/v0.6.6 Jul 30, 2025
20 of 21 checks passed
@urmauur urmauur deleted the fix/download-progress branch July 30, 2025 11:36
@github-project-automation github-project-automation bot moved this from Needs Review to QA in Jan Jul 30, 2025
@urmauur urmauur moved this from QA to Done in Jan Jul 30, 2025
@urmauur urmauur linked an issue Jul 31, 2025 that may be closed by this pull request
3 tasks
@urmauur urmauur moved this from Done to QA in Jan Jul 31, 2025
@louis-jan louis-jan mentioned this pull request Jul 31, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

bug: model download progress bar on the bottom-left not visible

5 participants