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

feat: clamp title and hint size to min 20% width #668

Merged

Conversation

christoph-heinrich
Copy link
Contributor

@christoph-heinrich christoph-heinrich commented Oct 4, 2023

Doesn't yet have any of the non-linear stuff, but it's already a big improvement.

ref #665 (comment)

For comparison with the screenshot in the comment:
image

@christoph-heinrich christoph-heinrich changed the title feat: clamp title and hint size to min 10% width feat: clamp title and hint size to min 20% width Oct 4, 2023
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Oct 4, 2023

I've tried doing a non linear interpolation together with that minimum size thing, and it adds a bunch of complexity to the calculation while also being susceptible to text with inaccuracies. It also has to account for the possibility that due to the non linearity one part actually gets much more space then it actually needs and so that then requires a clamp with the text width.

Overall it wasn't easy to get right, or rather I though it wasn't right due to the text width inaccuracy and took a bunch of debugging to make sure it's actually correct, and the benefit over a simple linear interpolation with clamping is barely noticeable. Maybe with 100% accurate text widths it would be a worthwhile improvement, but as things stand I actually prefer the results of clamped linear interpolation.

In case this ever becomes relevant in the future, I took half of a 2 wide Hann function and transformed it from y(x) = to x(y) =, which results in math.acos(1 - 2 * y) / math.pi

@tomasklaen
Copy link
Owner

tomasklaen commented Oct 4, 2023

How about doing something like:

If the delta in width between title and hint is less than 30% of the total width they require (they're around same lengths), we use the current linear cutoff.

If it's bigger, we let the shorter one decide the cutoff so that its text is never clipped.

Because the longer the text is, the less of an issue it is to clip something out of it. For example in the inputs menu, I wouldn't mind not seeing the whole command, I can already infer what it does from the visible part, but I would mind getting the shortcut clipped.

Also there should be some spacing between title and hint clips.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Oct 4, 2023

Here it is, but I don't like it.
As the size of the menu changes, so does the decision for the clipping and as a result it jumps when resizing.
image
And after making it a few pixels smaller.
image

Notice how the gap jumped to the right, and also changed in size due to text width inaccuracies.

I also pushed an alternative that doesn't jump around and it generally works better imo.

However both have a major flaw, and that is that both title and hint can be very long, but differ in length enough to go into the "smaller one dictates cutoff" path, in which case it could be that there is nothing left of the larger one, and the smaller one even clips outside of the menu.

To fix that we'd have to clamp it to make sure there is at least something left of the longer one, in which case we effectively end up with the same thing as I started off here, but the other way around (the smaller one gets more space then the larger one).

I still think the best thing is the one I started out with. We can increase the percentage if you think it's not enough, but the alternatives all suck in some way.

@tomasklaen
Copy link
Owner

I really don't want long texts to cause clipping of really short ones. If text is long, it has a lot of noise and therefore is a prime candidate for clipping. If it's short, it's dense with information, and shouldn't be clipped.

So we need to find some ideal conditions to trigger this mode. Lets say the text can't be clipped, and all clipping happens on the other one if it's:

  • smaller than half of the other one
  • and smaller than 120 pixels

If neither satisfies this, we just use linear clipping we have now.

@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Oct 4, 2023

  • smaller than half of the other one
  • and smaller than 120 pixels

I'd rather not set a hard limit like 120 pixels, and instead make it relative to menu width or something.

I've now changed it to a 50% cap, so each one can only use more then that if the other one doesn't need it.
That will provide those 120 pixels or more for menus down to 240 pixels (actually a bit more for spacing and padding), which is still a relatively small menu.

There has got to be a smarter way of writing the same thing, but I can't think of anything...

@tomasklaen
Copy link
Owner

both are entitled to half

Yeah I really like this solution 👍.

@tomasklaen tomasklaen merged commit 2f46e23 into tomasklaen:main Oct 7, 2023
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