Opened 9 years ago

Last modified 4 years ago

#6882 assigned bug

BTextControl layout issues

Reported by: axeld Owned by: yourpalal
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

While fixing bug #6863 I came across the following bugs:

  • setting a min size on the text view layout item does change the size of the text control, but does not affect the size of the text view, ie. if you make the layout item smaller, the text view will go out of the visible area.
  • the layout item contains 5 pixels spacing for the label - this should be done via layout spacing, not being hard-coded.
  • preferred size is broken (100 pixel width, how so?)

Change History (7)

comment:1 Changed 9 years ago by luroh

#6015 looks like a possibly related ticket.

comment:2 in reply to:  description ; Changed 9 years ago by bonefish

Milestone: R1R1/beta1
Owner: changed from axeld to yourpalal
Status: newassigned
Version: R1/alpha2R1/Development

Replying to axeld:

While fixing bug #6863 I came across the following bugs:

  • setting a min size on the text view layout item does change the size of the text control, but does not affect the size of the text view, ie. if you make the layout item smaller, the text view will go out of the visible area.

Did you try to call InvalidateLayout() on the view? The BAbstractLayoutItem::SetExplicit*() calls don't do that. Don't know if for a reason.

  • the layout item contains 5 pixels spacing for the label - this should be done via layout spacing, not being hard-coded.

That would be nicer indeed. The current implementation just maps to the facilities of the non-layout version and there is no settable spacing. If we want to change that, we should do so before R1, since it breaks layouts relying on how it works now.

  • preferred size is broken (100 pixel width, how so?)

This is actually a tricky one. The obvious approach would be to compute the preferred width based on the text width. The problem is that it changes while typing, so the whole layout would need to be invalidated all the time. My only idea for compensating for that would be a more elaborate layout invalidation mechanism (track the different properties (min/max/preferred size, alignment) individually (e.g. in a flags array per layout/view)).

Assigning to Alex in case he's interested.

comment:3 in reply to:  2 ; Changed 9 years ago by axeld

Replying to bonefish:

Did you try to call InvalidateLayout() on the view? The BAbstractLayoutItem::SetExplicit*() calls don't do that. Don't know if for a reason.

I did not try that, actually. However, since the views weren't even added to a layout, I'm not sure if that would have had any effect. Shouldn't you know at least if there is a reason that they don't call it, though? :-)

  • the layout item contains 5 pixels spacing for the label - this should be done via layout spacing, not being hard-coded.

That would be nicer indeed. The current implementation just maps to the facilities of the non-layout version and there is no settable spacing. If we want to change that, we should do so before R1, since it breaks layouts relying on how it works now.

I guess this would only come into play when there is no parent layout involved? This would also finally deprecate the SetDivider() stuff (which should then have no effect anymore).

  • preferred size is broken (100 pixel width, how so?)

This is actually a tricky one. The obvious approach would be to compute the preferred width based on the text width. The problem is that it changes while typing, so the whole layout would need to be invalidated all the time. My only idea for compensating for that would be a more elaborate layout invalidation mechanism (track the different properties (min/max/preferred size, alignment) individually (e.g. in a flags array per layout/view)).

Ah, indeed. One solution (that would follow what Java does, for example) would be not to invalidate the layout on SetText() - this way, the size would only actually matter when the layout is manually invalidated. I am not saying I like this solution, though :-)

comment:4 in reply to:  3 Changed 9 years ago by bonefish

Replying to axeld:

Replying to bonefish:

Did you try to call InvalidateLayout() on the view? The BAbstractLayoutItem::SetExplicit*() calls don't do that. Don't know if for a reason.

I did not try that, actually. However, since the views weren't even added to a layout, I'm not sure if that would have had any effect.

If I understand you correctly (you added the views to a layout only after setting the explicit size), then, yes it wouldn't have any effect.

Shouldn't you know at least if there is a reason that they don't call it, though? :-)

I haven't touched that code for more than four years. What do you expect? :-)

  • the layout item contains 5 pixels spacing for the label - this should be done via layout spacing, not being hard-coded.

That would be nicer indeed. The current implementation just maps to the facilities of the non-layout version and there is no settable spacing. If we want to change that, we should do so before R1, since it breaks layouts relying on how it works now.

I guess this would only come into play when there is no parent layout involved?

If we want to add a public SetSpacing(), it shouldn't have any effect when using the layout items. Though, I wasn't even considering to add such a method yet. I was only thinking of adding a respective attribute.

This would also finally deprecate the SetDivider() stuff (which should then have no effect anymore).

This isn't related. When using the layout items, both the explicitly set spacing and divider are ignored, otherwise both are used.

I may sound like a broken record, but BTextControl and BMenuField are also on the list of interface kit class that need to be rewritten from the scratch. The layout items solution is just a work-around for the label/divider mess. The R2 interface kit should have a separate BLabel class.

  • preferred size is broken (100 pixel width, how so?)

This is actually a tricky one. The obvious approach would be to compute the preferred width based on the text width. The problem is that it changes while typing, so the whole layout would need to be invalidated all the time. My only idea for compensating for that would be a more elaborate layout invalidation mechanism (track the different properties (min/max/preferred size, alignment) individually (e.g. in a flags array per layout/view)).

Ah, indeed. One solution (that would follow what Java does, for example) would be not to invalidate the layout on SetText() - this way, the size would only actually matter when the layout is manually invalidated. I am not saying I like this solution, though :-)

Nah, that's ugly. Either the text directly influences the preferred width -- then text changes must invalidate the layout -- or it doesn't. Otherwise one gets unpredictable effects for layouts that use the preferred sizes of their items (we don't have any ATM, but a "flow" layout would do exactly that). IMO an acceptable solution would be to compute the preferred size only from the initial text and keep it after that. Maybe invalidate and recompute it on certain events (e.g. font changes or when the view is attached to a window).

comment:5 in reply to:  2 Changed 8 years ago by yourpalal

Replying to bonefish:

Replying to axeld:

While fixing bug #6863 I came across the following bugs:

  • setting a min size on the text view layout item does change the size of the text control, but does not affect the size of the text view, ie. if you make the layout item smaller, the text view will go out of the visible area.

Did you try to call InvalidateLayout() on the view? The BAbstractLayoutItem::SetExplicit*() calls don't do that. Don't know if for a reason.

It seems to me that they probably should, BAbstractLayout::SetExplicit*() methods don't either, although they at least call BView::SetExplicit*() in some cases, which does cause a layout invalidation.

  • the layout item contains 5 pixels spacing for the label - this should be done via layout spacing, not being hard-coded.

That would be nicer indeed. The current implementation just maps to the facilities of the non-layout version and there is no settable spacing. If we want to change that, we should do so before R1, since it breaks layouts relying on how it works now.

  • preferred size is broken (100 pixel width, how so?)

This is actually a tricky one. The obvious approach would be to compute the preferred width based on the text width. The problem is that it changes while typing, so the whole layout would need to be invalidated all the time. My only idea for compensating for that would be a more elaborate layout invalidation mechanism (track the different properties (min/max/preferred size, alignment) individually (e.g. in a flags array per layout/view)).

Assigning to Alex in case he's interested.

Thanks, it's now on my todo list :)

comment:6 Changed 4 years ago by richienyhus

have there been any improvements in the interim?

Could we move this over to beta2 ?

comment:7 Changed 4 years ago by pulkomandy

Milestone: R1/beta1R1
Note: See TracTickets for help on using tickets.