Fix bugs with appTabs render

[spxvszero]

Description:

Fixes #3980

I also fix the appTabs indicator and seperator line does not show properly.

you can compare with this demo code :

func main() {
	a := app.New()
	w := a.NewWindow("Hello")

	closeTab1Btn := widget.NewButton("Close Tab1", nil)
	tab := container.NewAppTabs(
		container.NewTabItem("Tab 1", container.NewVBox(widget.NewLabel("Content of tab 1"),
			closeTab1Btn)),
	)
	closeTab1Btn.OnTapped = func() {
		tab.RemoveIndex(0)
	}

	left := container.NewVBox(widget.NewButton("Add", func() {
		btn := widget.NewButton("Close", nil)

		item := container.NewTabItem(fmt.Sprint(rand.Int()),btn)

		btn.OnTapped = func() {
			tab.Remove(item)
		}
		tab.Append(item)
	}))

	split := container.NewHSplit(left, tab)
	split.Offset = 0.2

	w.SetContent(split)
	w.Resize(fyne.NewSize(640,480))
	w.ShowAndRun()
}

Checklist:

• Tests included.
• Lint and formatter run with no errors.
• Tests all pass.

[andydotxyz]

Thanks for this @spxvszero but you'll need to fix the failures in CI before it will get reviewed

[coveralls]

coverage: 66.18% (+4.3%) from 61.907% when pulling 185cf55 on spxvszero:fix_apptabs into 93998ba on fyne-io:develop.

[ismyhc]

@spxvszero could you request review if you are ready so we can hopefully get this merged :) Id like to see these fixes in something I'mbuilding.

[Jacalz]

I'm sorry that this has taken some time to review. It is indeed a good idea to request a re-review (top right on the little circling arrows) once you feel happy enough with the code to do so. Commits can go unnoticed depending on notification settings here on GitHub. It makes it a bit easier for us to know that we should have a look at it again :)

[Jacalz]

Thanks for this. I just have a minor comment about keeping the code in line with our preferred code style. Apart from that, it looks good to me

[andydotxyz]

Are you sure the divider should be hidden?
Just because there are no tabs does not mean that this widget becomes invisible. Won't the MinSize still leave room for it?

[spxvszero]

@andydotxyz I am pretty sure this divider should be hidden, show you the picture, it looks better after hidden the divider.

And the tabs minSize calculation does not contains divider, so i think it is ok for doing this.

fyne/container/tabs.go

Lines 367 to 399 in 7996862

	func (r *baseTabsRenderer) minSize(t baseTabs) fyne.Size {
	pad := theme.Padding()
	buttonPad := pad
	barMin := r.bar.MinSize()
	tabsMin := r.bar.Objects[0].MinSize()
	accessory := r.bar.Objects[1]
	accessoryMin := accessory.MinSize()
	if scroll, ok := r.bar.Objects[0].(*Scroll); ok && len(scroll.Content.(*fyne.Container).Objects) == 0 {
	tabsMin = fyne.Size{} // scroller forces 32 where we don't need any space
	buttonPad = 0
	} else if group, ok := r.bar.Objects[0].(*fyne.Container); ok && len(group.Objects) > 0 {
	tabsMin = group.Objects[0].MinSize()
	buttonPad = 0
	}
	if !accessory.Visible() || accessoryMin.Width == 0 {
	buttonPad = 0
	accessoryMin = fyne.Size{}
	}
	contentMin := fyne.NewSize(0, 0)
	for _, content := range t.items() {
	contentMin = contentMin.Max(content.Content.MinSize())
	}
	switch t.tabLocation() {
	case TabLocationLeading, TabLocationTrailing:
	return fyne.NewSize(barMin.Width+contentMin.Width+pad,
	fyne.Max(contentMin.Height, accessoryMin.Height+buttonPad+tabsMin.Height))
	default:
	return fyne.NewSize(fyne.Max(contentMin.Width, accessoryMin.Width+buttonPad+tabsMin.Width),
	barMin.Height+contentMin.Height+pad)
	}
	}

[andydotxyz]

No other widgets exhibit the behaviour of appearing invisible if they have no content. I wonder what the use-case is for the bar disappearing if items are removed?

[ismyhc]

I agree it looks much better with the bar hidden, but my question is why would you ever have no tabs in this widget? In the case of not having tabs, it should probably be up to the user to hide the apptabs widget I think.

[Jacalz]

Yeah, no tabs in a tab container seems a bit strange. Most applications that I know of close the whole window if you close the last tab. That, or just hiding the tab widget itself if necessary, seems like something the developers can do already.

[andydotxyz]

In my applications I make that a "Welcome" tab. That assumes people would like to go back to it of course.
I also wonder if you are describing DocTabs where user can add and remove tabs themselves, AppTabs are considered more static (developer controlled app sections).

Perhaps the "right" fix is that the line should not only remain visible but the bar should not collapse in height as it is currently doing.

[ismyhc]

Any chance you were able to finish this up @spxvszero ? Sorry to bug, but would really like to see this get merged in as the current bug on develop looks terrible.

I guess I could just use your fork instead. lol

[Jacalz]

It could be wise to separate out the bug fix and the behaviour change. I think it should make this land more quickly

[spxvszero]

@andydotxyz @ismyhc @Jacalz Sorry for replying so late. I think we cannot figure out all the use case and stand for their perspectives. But we can make it looks more natural and comfortable, so i did these change. I agree with @Jacalz , i should separate out the bug fix and the behavior change, i discard the behavior change.Thanks for reviewing.

[ismyhc]

Looks good to me. @spxvszero you may need to request review from @andydotxyz or @Jacalz

[ismyhc]

This looks good. As suggested leaving the behavior changes for another request.

[andydotxyz]

Great thanks

[Jacalz]

FYI: I'll review and merge this later today.

[Jacalz]

This looks great. Thanks for the contribution and congratulations on landing your first PR to Fyne. I also want to add that I especially appreciated the test code that you added to the description. It made it nice and easy to review the bug fix part of this :)