Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor UI changes: Filename only, tooltip, and scrolling #73

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

JedBurke
Copy link
Contributor

This pull request contains a few minor user-facing changes:

  • CLI switch to only show the filename, not the full path (enabled with -f/--name-only).
  • Show a tooltip containing the full path when hovering over a file.
  • Use gtk_scrolled_window_new to allow scrolling when there are many items.

They've been kept separate as to cherry pick the desired features.

@@ -551,6 +558,9 @@ int main (int argc, char **argv) {

window = gtk_window_new(GTK_WINDOW_TOPLEVEL);

// Scrolling Window
panel = gtk_scrolled_window_new(NULL, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this panel's size determined? As large as can be without exceeding monitor dimensions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will have to give it a look. The widget plays nicely in i3 with titling mode (takes the full height) but not in floating.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How is this panel's size determined?

It takes the size of the parent window. So if the default size is set, gtk_window_set_default_size(GTK_WINDOW(window), 600, 500) the scroll window will occupy that space.

As large as can be without exceeding monitor dimensions?

Might be dependent on the user's desktop environment. Testing on i3 with floating mode, the window size is curbed when setting the default size (with gtk_window_set_default_size) to a height greater than the monitor.

Copy link
Owner

Choose a reason for hiding this comment

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

It seems that this results in the smallest possible non-resizable square window (but scrollable). It needs better logic for determining a size, probably only scrolling if it can't fit the contents. I don't know how to make that work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yesterday I had a look to set the default size according to the user's screen dimensions without hard coding it. gtk_window_set_geometry_hints with a comfortable hard coded minimum could prevent the window from being too small unless overridden by gtk_window_set_default_size which was looking to be the result of gdk_monitor_get_workarea.

probably only scrolling if it can't fit the contents

That's a better idea. The scrollbars already show when necessary. Again, without setting the default window size, gtk_scrolled_window_set_min_content_width and gtk_scrolled_window_set_min_content_height will set the top-level window's size.

With regards to a dynamic size. Not sure if it's possible, but how about summing the height of each button and using it for the window if it's lower than the working area? The same would go for the width, select the widest button and use its width if it's less than the working area width.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it's possible, but how about summing the height of each button and using it for the window if it's lower than the working area?

It's simpler than that, after calling gtk_widget_show_all(window);, the allocated width and height of vbox can be used. Its height and width can exceed the workarea, so they need to be curbed before calling the respective scrolled_window functions.

Additionally, gtk_window_set_geometry_hints can be used to ensure a comfortable minimum size if vbox is too small. In the example below, the left has no minimum size while right has a minimum of 640x480.
comparison

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this was just an example, but please do not make the window larger than it needs to be. I frequently use dragon with single items and would prefer those not to use 500px in height when 80 would have sufficed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this was an example of how to control the size. I will take it under advisement and not enable it.

dragon.c Outdated
@@ -160,6 +160,9 @@ GtkButton *add_button(char *label, struct draggable_thing *dragdata, int type) {
button = gtk_button_new_with_label(label);
}

// Show a tooltip with the filename. Should perhaps show the full path.
gtk_widget_set_tooltip_text(button, label);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good idea. I'd even argue that with the tooltip the "name only" option could become the default behavior, perhaps even the only one, but the option is cheap to implement anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you feel like it can become the default, that's good. The option was included as not to cause disruptions for longtime users. So it can be removed if unnecessary.

Copy link
Owner

Choose a reason for hiding this comment

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

I think the ideal default is probably something like "match name given on command line, if any", so that e.g. dragon */index.html preserves the relative path to distinguish items with the same basename, but wouldn't convert everything to absolute paths the way it does now; the architecture makes that tricky currently, I think. There could then be options for basename-only (this commit) and absolute paths-only (current default). This change is a good step for common cases now and it would be a reasonable default for as long as we don't have that middle point. The tooltip makes it possible to distinguish them in the cases where it matters.

Perhaps -f like this, -F/--full-path for the existing default, and switch by default to the -f behaviour for the moment?

Copy link
Owner

Choose a reason for hiding this comment

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

I have made it do something like that, at least when the path is within the current directory. It could probably be smarter, but it's a start. Thanks for the PR! These were useful. The tooltip is a great idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adapting to the working directory is a clever compromise and functions well.

When showing the filename only, the tooltip doesn't have access to the full path. Instead of setting the tooltip in add_file_button and add_uri_button, I've added a parameter to add_button to be able to set the tooltip to the path. Please see if it's an acceptable change 9c80b7e

It could probably be smarter, but it's a start.

When you say smarter, do you mean that it should show the basename and then resolve duplicates by showing the file's parent and its basename?

Consider the working directory /path/to/file/project-1

Before After
./file-a.txt file-a.txt
./file-b.txt project-1/docs/file-b.txt
/path/to/file/project-2/file-b.txt project-2/docs/file-b.txt
/tmp/file-b.txt /tmp/file-b.txt

Thanks for the PR!

You're welcome

@mwh
Copy link
Owner

mwh commented Dec 26, 2024

Other than the scroll window that doesn't seem usable without something to impose a window size, these all look good to me. I've cherry-picked the other commits, fixuped the space one, and switched the default button to "relative path to ." with this -f option and a new -F to switch between basename-only (this PR's -f) and absolute-only (previous default). That is all merged now.

Scrolling is probably useful for large lists/small windows (especially in a tiling environment), but we need to have the program work "as normal" in standard cases as well and that doesn't happen yet. If there's a clear way to make it auto-size as now if it can fit, that will be good.

When adding a file button and enabling the filename only option, the
`filename` variable becomes unsuitable for obtaining the pixel buffer
from the file.
The tooltip needs to display the file's path, but will be unavailable if
the label is passed as the basename only. If the tooltip value is null,
default to the label input.
@JedBurke
Copy link
Contributor Author

Commits 5b48527 and 9c80b7e explain themselves. 31a53a2 sets the window geometry based on vbox's size (which is obtained from its children) up to the working area. Maybe there is an overlooked function, but I wasn't able to set the window including its decorations to the working area. To avoid it extending past the workarea, it will be shaved by 5%.

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.

3 participants