dreamcat4 / skippy-xd

A full-screen Exposé-style standalone task switcher for X11.
GNU General Public License v2.0
100 stars 12 forks source link

truncate long tooltips #10

Closed vredesbyyrd closed 1 year ago

vredesbyyrd commented 4 years ago

Windows with long titles can result in tooltips that are more distracting than helpful, imho.

Screenshot from 2020-03-12 22-26-40

Maybe a solution would be to truncate tittles that are greater than its preview width? That would at least prevent tooltips from overlapping other previews. I believe that's how gnomes 'overview' handles it. Although I have not used gnome for a few years so i'll have to take a look.

Thanks for all your work maintaining this great tool. I would not have been able to move from gnome to budgie without it :)

dreamcat4 commented 4 years ago

Hey no need to thank me as work on it was never finished to my liking. For example it still segfaults.

What is good news is there are other seperate efforts in the budgie project for wanting improving the official wpreviews windows switching tool in vala. Which is something else also worth participating in.

For this tool, the tooltip text gets drawn onto the screen the old school X Windows way with some very vanilla and basic call to whatever xlib function it is. So whilst I cannot remember off the top of my head the location in the code where it occurs is a findable one.

There is a draw call for the mouse cursor, when it moved and that is when this tooltip draw call gets invoked. The text is then redrawn and updated....

Ok i have looked at the code and can say that the tooltip is shown by calling tooltip_map function in file clientwin.c. When x event is received to say that mouse has moved over a preview tile (= 'client window' of the main skippy window).

However to change the look of the tooltip must happen at an earlier time, during the tooltip_create function. Which is probably called only during skippy program setup launch time. 1 time per each sub window preview represented in skippy's linked list of items. During a linked list traversal.

So that would be in the file tooltip.c.

Unfortunately I am too busy to look into this. However I really encourage others to try improve this part of skippy as it has merit and would improve skippy overall program usability. It is also a good introduction to skippy as a way in for new developer. Without the learning curve being initially too hard.... although it does tend to get harder very soon after branching out ans expanding away from that. Due to skippys rather eccentric combined data and state representations and its archaeic event driven and non linear execution flow. Anyhow. Not to get into that here and scare people away. Its a beautiful programming challenge! A bit like weeding an unkempt and very much derelict / abandoned garden. Around a set of ruins of whatever it once was. [shudders].

I'd be happy to accept PRs for such related feature to improve the tooltip appearance and operation. And to encourage other developers to get more involved with skippy.

vredesbyyrd commented 4 years ago

What is good news is there are other seperate efforts in the budgie project for wanting improving the official wpreviews windows switching tool in vala. Which is something else also worth participating in.

Good to hear, I will have to take a look at that.

Appreciate the rundown on skippy's tooltip code. Although I am very inexperienced with c. The furthest I have went has been very surface level stuff, writing a few basic command line programs as learning exercises. But I have been wanting to dig in and really learn c for a awhile now. Perhaps trying to tackle this issue could be a small way towards that. Its looking like I may have some free time coming up, so I just might test my patience and see if I can wrap my head around this area of skippy :)

Thanks again for the rundown! I'll make a PR if I can implement a nice solution.

ghost commented 4 years ago

Hi, I have looked a little bit into this and found a solution to limit the tooltip width to its preview width. See #12, what do you think about it?

dreamcat4 commented 4 years ago

Have already merged it. Let me know how it looks! Kind regards.

ghost commented 4 years ago

Screenshot_20200411_223242

dreamcat4 commented 4 years ago

Ah thanks for the screenshot. It was very helpful to better understand the effect.

Hey i think you improved things by adding that new function parameter to the arguments. However perhaps in the situation of many preview client windows (instead of just only 4 windows pictured above)... then each preview window width will become very small indeed. And so that same variable will cut the tooltip text far too much. So the chosen variable is not the right guide. I would therefore instead suggest to make it something else. In then absence of a proper metric based on the tooltip font size etc. then the main window width (or display width) x a fraction float such as 0.3 or other similar constant. Would ensure the tooltip it clipped at a constant 30% of the total screen width for that monitor. This constant could be adjusted a bit based on how it looks.

Another way to go would be to still use your original preview clientwin width variable. But then in addition to that define some maximum and minimum width limits for the tooltip as an additional constraint. In particular the minimum width. Or is there already such a constraint already applied, that I have not noticed yet or overlooked? My time studying this is not enough to examine it so deeply to check up on such things. Anyhow

Perhaps a further (and more challenging) improvement after that would be to break a multi line text string after N chars. And to write a small helper function that broke the lines up into a char** array of strings intelligently on word boundaries. Such a function would also require memory management use / free at the end of the life cycle for those pointers.

Hopefully you can understand these recommendations, although I have not had the time to look into this matters myself I trust you might like to continue your efforts here to want to improve this feature. Look forwards to hearing from you again perhaps with more PRs. Until next time!! Good day.

ghost commented 4 years ago

I did not think about that preview width could be very small and agree that using clientwin width is not a good idea then.

I think mainwin width * factor is the better approach than adding an additional minimum width. How would we choose that minimum width? Perhaps using mainwin width * factor too. But then we can use the calculated minimum directly to decide if we truncate the tooltip. Also, If I am not mistaken, it is possible that the text is too long to fit in clientwin width but too short to fit nicely (without trailing empty space) into the forced minimum width.

So I choosed to go the mainwin width * factor way in #13.

This is how it looks with factor 0.3 on my 1920x1080 monitor: Screenshot_20200412_150929

... and with factor 0.4 Screenshot_20200412_151206

dreamcat4 commented 4 years ago

Yes you would only need to enforce a min width if you had continued to go with the variable clientwin sizes. Which can end up being very small.

Since you didn't do that then it probably doesnt matter. Since it's unlikely that anybody is viewing skippy on tiny displays that are less than about 300px wide. Which then would end up making the tooltip < 100px wide after the 0.3 scaling factor.

vredesbyyrd commented 4 years ago

@arisinfenix Thank you for implementing this :)

If you are not against making further changes, a visual cue to the user that a tooltip is truncated could be a nice addition, perhaps an ellipsis at the end of the shortened strings?

I logged into gnome to see how they handle it:

Screenshot from 2020-04-11 21-55-04

ghost commented 4 years ago

@vredesbyyrd I like your idea. :) I have implemented adding ... at the string end, but will do some more testing before posting it.

ghost commented 4 years ago

Opened a PR now, below is a screenshot with the changes from #14 applied.

Screenshot_20200413_174302

dreamcat4 commented 4 years ago

Thanks for this! Merged.

Just some quick notes / feedback for future - if you want to add more inline comments than was in keeping with the original code. Or if you ever feel like cleaning up little bits and pieces nearby as you go... Then that is probably usually ok and fine with me. But regardless of those incidental matters I am very happy to take your contributions here on an 'as is' basis.

For this issue I shall leave this open for a while. To see what you guys all think and can decide some consensus if its already enough then of course we can close. Or if you all want to take it any further. And slowly keep building upon that with additional incremental enhancements. I am happy to leave it in your hands either way. I guess it really depends where you are finding the most fun is to be found.

But I must say the new functionality of today seems quite nice. Well done!

vredesbyyrd commented 4 years ago

Testing the latest commit and all seems good. The added ellipsis gives the truncated strings some polish, imo :)

The absence of excessively wide tooltips is very helpful for me - I have a lazy eye so aesthetically "loud" ui elements often cause me to see double 🥴 . With skippy that should not be an issue anymore. Your work is very appreciated!