Skip to content

Introduce side-widget in Abstract Slot and use for preview window#2714

Merged
danirabbit merged 39 commits intomainfrom
jeremypw/details-column/reposition
Apr 23, 2026
Merged

Introduce side-widget in Abstract Slot and use for preview window#2714
danirabbit merged 39 commits intomainfrom
jeremypw/details-column/reposition

Conversation

@jeremypw
Copy link
Copy Markdown
Contributor

@jeremypw jeremypw commented Jan 29, 2026

Fixes #2696

Screenshot from 2026-01-29 19 26 19

Instead of inserting preview widget into slot stack in Miller it is added as a side-widget to the main AbstractSlot (will also work for other views). This minimises impact on the Miller view and makes it easier to fix some issues.

The side-widget is added into a Paned so the space it takes up can be adjusted by the user.

The preview size has been reduced to be more comparable to the directory slots but wide enough not to cause undue wrapping of the property information

The preview size was chosen to minimise the appearance of black bars to the sides (sometimes) when the window height is changed. I think this may be a window manager issue?

The Miller View now keeps the active slot in view when the space available to it changes either due to window resizing or preview widget space changing.

It would be nice if the width of the active slot were kept above a minimum but that applies to main anyway so can be done in a separate PR.

@jeremypw jeremypw requested a review from danirabbit January 29, 2026 19:43
@jeremypw
Copy link
Copy Markdown
Contributor Author

@danirabbit There's probably some clean-up and refinement to do but I would appreciate some feedback before going any further.

@jeremypw jeremypw mentioned this pull request Jan 29, 2026
3 tasks
@danirabbit
Copy link
Copy Markdown
Member

danirabbit commented Jan 30, 2026

Yeah this seems like a much better direction. Nice job!

I've got some weird gap here added to the end of the scrollable area:
Screenshot from 2026-01-30 10 45 38

@jeremypw
Copy link
Copy Markdown
Contributor Author

jeremypw commented Jan 31, 2026

@danirabbit Thanks for the review! I am encouraged to spend more time on it. That gap has always been there in Miller View as a consequence of the need to have space for the column to expand into (see calculate_total_width function), but before it lay outside the window. I'll see what I can do to hide it.

@jeremypw
Copy link
Copy Markdown
Contributor Author

jeremypw commented Jan 31, 2026

@danirabbit I have got rid of the gap now but you can reveal the expand-into space by reducing the preview width with the paned (or window larger or deselecting) if you do need to make the last slot larger. Whenever a "scroll to slot" is performed the gap disappears.

I have also made other minor refinements to scroll handling.

I'll dog-food it for a while but if you see anything let me know.

# Conflicts resolved:
#	libcore/AbstractSlot.vala
#	plugins/pantheon-files-trash/plugin.vala
#	src/View/Miller.vala
@jeremypw
Copy link
Copy Markdown
Contributor Author

@danirabbit Further releases of Files are pretty much blocked until the preview facility is acceptable. I would be grateful for your input on this PR which, I think, fixes a number of issues with the current situation.

@jeremypw jeremypw requested a review from a team March 2, 2026 15:31
@danirabbit
Copy link
Copy Markdown
Member

I feel like we've gone better in a lot of ways and worse in some ways. Can you revert any changes you've made to the design of the contents of the pane? So changes to scrolled areas, adding the separator etc. I'd like to consider those kinds of changes separately

@jeremypw
Copy link
Copy Markdown
Contributor Author

jeremypw commented Mar 7, 2026

@danirabbit Not sure exactly what you want but I have reverted all changes made to the DetailsColumn widget since moving to side widget.

@jeremypw jeremypw added this to the 7.3.0 milestone Mar 19, 2026
@jeremypw jeremypw mentioned this pull request Apr 18, 2026
2 tasks
@jeremypw
Copy link
Copy Markdown
Contributor Author

@danirabbit Any chance of having another look at this? It is needed for the next release.

Copy link
Copy Markdown
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Thanks for the ping :) Just a few code comments otherwise this looks good. Sorry for taking a while to review!

Comment thread libcore/AbstractSlot.vala Outdated
Comment on lines +52 to +53
protected Gtk.Grid content_box;
protected Gtk.Paned side_widget_box;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we make sure to name this in a way that indicates what they are. These names imply that they're Gtk.Box. It should instead be content_grid and side_widget_paned I think

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Better names now used

Comment thread libcore/FileUtils.vala
Comment on lines +649 to +651
if (!pad) {
return s;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't see where this is ever set false?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I clearly intended an option not to pad the datetime but I cannot remember what situation I was thinking of. Anyway it is not used in this PR (now) so I'll remove.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh, I remember now - we need to not pad the datetime in the DetailsColumn. I will how actually implement that!

Comment thread src/View/Miller.vala Outdated
guest.slot_number = (host != null) ? host.slot_number + 1 : 0;
guest.colpane = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
guest.colpane.set_size_request (guest.width, -1);
// guest.colpane.set_size_request (guest.width, -1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some commented code here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Now gone

Comment thread src/View/Miller.vala Outdated
Comment on lines +124 to +126
guest.colpane.set_size_request (
2 * guest.width, -1
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use just width_request since the height request is -1?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread src/View/Miller.vala Outdated
total_width += slot.width;
});

this.colpane.set_size_request (total_width, -1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Another place where we can probably use width_request instead of set_size_request

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@jeremypw jeremypw requested a review from danirabbit April 23, 2026 16:56
@danirabbit danirabbit merged commit 88b38d7 into main Apr 23, 2026
4 checks passed
@danirabbit danirabbit deleted the jeremypw/details-column/reposition branch April 23, 2026 18:56
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.

Column preview gets cut off

2 participants