CPTabView insertTabViewItem:atIndex: does not work #1920

Closed
Alos opened this Issue May 6, 2013 · 11 comments

Projects

None yet

6 participants

@Alos

The problem

CPTabView insertTabViewItem:atIndex: is not working correctly when items are added between other tabs. The resulting tab's sizes are not updated:

screen shot 2013-05-04 at 11 30 02 pm

The probable cause

The problem, I think its caused by the way tabs are inserted:

After adding the CPTabViewItem to the _items array the CPTabView calls _updateItems:

This method increments the segment count (this adds a new element at the end) and then iterates over the segments renaming them.

The problem is that renaming only triggers a resizing of the segment when its width is zero. (See here) The issue with this, is that if the element you are inserting is in the middle it will not get its size recalculated, since this is not a new element and it already has a width (The new element was added at the end).

This might be related to issue #301

A solution?

I can think of several solutions, one of them might be to make CPSegmentedControl be able to insert the new element where you need it. Another one might be to have CPSegmentedControl recalculate the size of every element that gets its label changed.

@cappbot

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

@ahankinson

-#new
+bug
+AppKit

@cappbot

Milestone: Someday. Labels: AppKit, bug. What's next? A reviewer should examine this issue.

@cacaodev cacaodev added a commit to cacaodev/cappuccino that referenced this issue Feb 18, 2015
@cacaodev cacaodev CPSegmentedControl : -insertSegmentsAtIndexes: -removeSegmentsAtIndex…
…es: . WIP. Trying to fix #1920. Test in Manual/CPSegmentedControl/
a565849
@cacaodev

I've been trying to fix this by implementing -insertSegment:atIndex: in CPSegmentedControl. It's trivial to allow insertion/deletion in the collection of individual segments, but there is still a relayout bug -> see CPSegmentedControl manual test - button "remove selected segment".

The layout code in CPSegmentedControl is awfully complicated, I guess because it uses _CPImageAndTextView . Does someone familiar with this code have ideas about how to make it more simple ? Maybe by putting more layout stuff in the segment object.

@Dogild
Cappuccino member

Hi @cacaodev, I quickly took a look to this problem.

From what you have done, I saw two issues :

  • When you add a new segment, you don't tile it (you should call the method tileWithChangedSegment:) once you have added every segments.
  • Secondly, when you remove a segment, you should calculate the new frameOrigin of the right segments of the deleted segments. You should shift every segments to the left.

If you do

- (void)_layoutAfterSegmentCountChange
{
    var count = [_segments count];

    if (_selectedSegment >= count)
        _selectedSegment = -1;

    var thickness = [self currentValueForThemeAttribute:@"divider-thickness"],
        height = CGRectGetHeight([self bounds]),
        dividerExtraSpace = (count - 1) * thickness,
        widthOfAllSegments = 0;

    for (var i = 0; i < count; i++)
    {
        [self tileWithChangedSegment:i];
        widthOfAllSegments += [_segments[i] width];
    }

    [self setFrameSize:CGSizeMake(widthOfAllSegments + dividerExtraSpace, height)];
}

and comment the lines 726 to 732 it should work.

    /*if (!delta)
    {
        [self setNeedsLayout];
        [self setNeedsDisplay:YES];

        return;
    }*/

But this is not an optimize solution at all

@cacaodev

Ok, thanks Alex. I thought this methods meant "tile starting at segment". now it works.

@daboe01

i do not see this issue anymore. @cacaodev can we close?

@cacaodev

this is fixed.

@daboe01

+#works-for-me

@cappbot

Milestone: Someday. Labels: #works-for-me, AppKit, bug. What's next? Attempts to reproduce the problem described by this issue have failed to reveal any erroneous situation.

@cappbot cappbot closed this Jun 8, 2016
@cappbot

Milestone: Someday. Labels: #works-for-me, AppKit, bug. What's next? Attempts to reproduce the problem described by this issue have failed to reveal any erroneous situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment