Icon-O-Matic Properties Input Field entries out of scope

[roiredxsoto]

Good day,

Recent update of Icon-O-Matic resulted in the Properties panel behaving a bit strange, where input fields ar mostly hidden, and typing names for the elements becomes blind typing.

Attached is a screencapture of the Properties section where the name of the shape can be changed but the text extends way out of sight (is this related to similar issue in Tracker?)

~> uname -a
Haiku hawku 1 hrev54507 Aug 17 2020 06:02:40 x86_64 x86_64 Haiku

Regards,
RR

[roiredxsoto]

Screen capture of the Properties panel of Icon-O-Matic

[roiredxsoto]

Possibly related to this one #16476?

[jscipione]

This is probably also related to hrev54496. The default insets on BTextView were changed so if Icon-O-Matic is doing some calculations that are assuming default insets are 0. Either we SetInsets to 0 or we undo the textview manipulations in Icon-O-Matic and go with the new default insets instead.

[jscipione]

I can confirm that hrev54496 is the culprit here. I have updated my revised patchset to fix this bug by going with the latter solution, removing the offending code and going with the default text view behavior. Calling SetInsets(0, 0, 0, 0); also fixes the problem.

The general problem here is code that looks like this:

void
MyTextView::FrameResized(float width, float height)
{
    BRect textRect(TextRect());
    textRect.right = textRect.left + width;
    SetTextRect(textRect);
}

This code falls down because the default insets get applied again and again as SetTextRect() is called shrinking the text rect until it can’t be seen anymore. So if this pattern is used elsewhere in apps we can’t update then we may not be able to make them work unless we set the default textview insets to 0.

[Starcrasher]

Make English behave like other languages wasn't an improvement? LOL
Seriously, left column is too thin and anyway once translated most languages ends up with cut text.
I think that in this specific case, the design is the real problem. Translators can try to make strings shorter and shorter but it has to remain understandable. At the end, if it is too small, there's nothing you can do.
No doubt that your observations will be helpful for other apps but, here IMHO the two solutions would be to reduce the drawing space or to make left bar float.

[pulkomandy]

Shouldn't the insets be added only if the text rect is too close to the view border? If getting a rect that's already somewhere in the middle of the view I don't see why we need to apply extra insets to it?

[jscipione]

The idea is that the insets get applied 1x insetting the text rect away from the view bounds. The offending code in Icon-O-Matic is to make the text rect wider as you type, which is something we are now doing in BTextView. Note that it doesn’t make the BTextView box any bigger, just the text rect inside so you have to scroll left/right to see all of the text in the text view bounds. I’m not trying to address the usability of the BTextView width here in different languages, I only want to fix the bug that is causing the text not to be displayed in the box correctly.

We don’t need this workaround in Icon-O-Matic anymore, and getting rid of it fixes the bug, but I still have a larger problem on my hands here because it seems like what Icon-O-Matic is doing ought to be allowed. The problem is we are applying the default insets to the text rect every time SetTextRect() is called.

[jscipione]

In my latest patch set [0] I have reverted all the Icon-O-Matic changes that I previously made and once again allowed Icon-O-Matic to work unmodified. The problematic code in Icon-O-Matic is a work-around for bugs that have now been fixed in BTextView so can be taken out... but I've changed BTextView such that Icon-O-Matic text views will continue to work even if we don't take the workaround out. See the code example above. This code has been ensured to work by only adding the default insets when the text rect is set to text view bounds. While this means that we may not get the default insets in some places where we want them, it also means that the insets will only get applied one time, and not over and over again. And not getting the default insets applied where they should in some places is not nearly as big a deal.

[0] ​https://review.haiku-os.org/c/haiku/+/3152

[roiredxsoto]

Good day,

It seems to be fixed with the latest hrev54531.

Regards,
RR

[jscipione]

The offending commit got reverted so this regression is fixed for the moment. However, I’m still working on BTextView changes, so let SSD keep this ticket open to track the regression through this process.

[jscipione]

Reverted in hrev54531. Fixed again in hrev54549.