Skip to content

Conversation

@Alxiice
Copy link
Contributor

@Alxiice Alxiice commented Jan 13, 2026

What does this PR do ?

It adds a list view to the image gallery :
list_view

Changes in the code

  • main changes :
    • the whole layout for the image gallery has been moved to specific files : ImageGridView and ImageListView. They are dynamically loaded through QML components with a loader depending on the layout mode
    • the delegate has been modified to be able to display different layouts (list layout mode is built as a row layout and grid mode is a column layout)
  • small improvements
    • search bar : when we press tag make sure it shows the search bar
    • when we press escape make sure it hides the search bar

Notes for the review

  • The one thing I didn't do is that I wanted write the code inside the view files (imagegridview/imagelistview) once and reuse the same component (because it's basically the same code but I had issues some with QML so for now the code is duplicated. If that's a big issue I can try to find a way around
  • Also I wanted to include the multi-image selection in this PR but it was a bit tricky so I will add this later

@codecov
Copy link

codecov bot commented Jan 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.80%. Comparing base (cca5cc5) to head (059075a).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2975   +/-   ##
========================================
  Coverage    79.80%   79.80%           
========================================
  Files           64       64           
  Lines         8674     8674           
========================================
  Hits          6922     6922           
  Misses        1752     1752           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a list view option to the image gallery, allowing users to switch between grid (thumbnail) and list layout modes for viewing images. The implementation refactors the gallery layout into separate, dynamically-loaded view components (ImageGridView and ImageListView) while updating the ImageDelegate to support both display modes.

Changes:

  • Refactored image gallery layouts into separate ImageGridView and ImageListView components with dynamic loading via QML Loader
  • Modified ImageDelegate to support both grid and list layout modes using conditional component loading
  • Enhanced SearchBar with Escape key handling to hide the search bar and improved visibility toggling

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
meshroom/ui/qml/ImageGallery/qmldir Registers the new ImageGridView and ImageListView components
meshroom/ui/qml/ImageGallery/ImageGridView.qml New grid view component extracted from main ImageGallery
meshroom/ui/qml/ImageGallery/ImageListView.qml New list view component with vertical layout for images
meshroom/ui/qml/ImageGallery/ImageGallery.qml Refactored to use dynamic loader for switching between views, added layout mode toggle
meshroom/ui/qml/ImageGallery/ImageDelegate.qml Updated to support both grid and list layouts with conditional rendering
meshroom/ui/qml/Controls/SearchBar.qml Added Escape key handler and improved visibility toggling
meshroom/ui/qml/WorkspaceView.qml Removed invalid QML syntax line
meshroom/core/init.py Split logging statement into two separate calls for better formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +395 to +385
Timer {
id: scrollTimer
interval: 50 // Small delay to ensure layout is complete
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The magic number 50 for the timer interval lacks context. Consider adding a comment explaining why this delay is necessary, or define it as a named constant like layoutInitializationDelay for better maintainability.

Suggested change
Timer {
id: scrollTimer
interval: 50 // Small delay to ensure layout is complete
// Delay (in ms) used to ensure the layout is fully initialized before updating
// the current index and attempting to scroll to the selected item.
readonly property int layoutInitializationDelay: 50
Timer {
id: scrollTimer
interval: layoutInitializationDelay

Copilot uses AI. Check for mistakes.
Comment on lines 246 to 248
width: layoutMode === ImageGallery.LayoutModes.Thumbnail ?
(layoutLoader.item ? layoutLoader.item.cellWidth : 160) :
(layoutLoader.item ? layoutLoader.item.width : 400)
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

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

The fallback width value of 400 appears to be a magic number without clear justification. Consider defining this as a named constant (e.g., defaultListViewWidth) or using a more contextual value based on the parent's width to ensure proper layout even before the loader completes initialization.

Copilot uses AI. Check for mistakes.
@Alxiice Alxiice force-pushed the dev/gallery_list_view branch 2 times, most recently from 7c7e410 to 577c407 Compare January 19, 2026 09:18
@Alxiice Alxiice force-pushed the dev/gallery_list_view branch from 1ac22ed to 059075a Compare January 21, 2026 10:58
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.

2 participants