Merge pull request #2117 from andydotxyz/fix/mobiledoctabs

Fix tab button layout on mobile
fyne-io · Apr 22, 2021 · 55fd860 · 55fd860
2 parents c9d4238 + 7dc2ac5
commit 55fd860
Show file tree

Hide file tree

Showing 11 changed files with 266 additions and 84 deletions.
diff --git a/container/apptabs.go b/container/apptabs.go
@@ -181,19 +181,7 @@ func (t *AppTabs) SetItems(items []*TabItem) {
 
 // SetTabLocation sets the location of the tab bar
 func (t *AppTabs) SetTabLocation(l TabLocation) {
-	// Mobile has limited screen space, so don't put app tab bar on long edges
-	if d := fyne.CurrentDevice(); d.IsMobile() {
-		if o := d.Orientation(); fyne.IsVertical(o) {
-			if l == TabLocationLeading || l == TabLocationTrailing {
-				l = TabLocationBottom
-			}
-		} else {
-			if l == TabLocationTop || l == TabLocationBottom {
-				l = TabLocationLeading
-			}
-		}
-	}
-	t.location = l
+	t.location = tabsAdjustedLocation(l)
 	t.Refresh()
 }
 
@@ -320,7 +308,11 @@ func (r *appTabsRenderer) buildTabButtons(count int) *fyne.Container {
 		if cells == 0 {
 			cells = 1
 		}
-		buttons.Layout = layout.NewGridLayout(cells)
+		if r.appTabs.location == TabLocationTop || r.appTabs.location == TabLocationBottom {
+			buttons.Layout = layout.NewGridLayoutWithColumns(cells)
+		} else {
+			buttons.Layout = layout.NewGridLayoutWithRows(cells)
+		}
 		iconPos = buttonIconTop
 	} else if r.appTabs.location == TabLocationLeading || r.appTabs.location == TabLocationTrailing {
 		buttons.Layout = layout.NewVBoxLayout()

diff --git a/container/apptabs_mobile_test.go b/container/apptabs_mobile_test.go
@@ -232,25 +232,25 @@ func TestAppTabs_Layout(t *testing.T) {
 			name:     "bottom: tab with icon only",
 			item:     container.NewTabItemWithIcon("", theme.InfoIcon(), canvas.NewCircle(theme.BackgroundColor())),
 			location: container.TabLocationBottom,
-			want:     "apptabs/mobile/layout_bottom_ico.xml",
+			want:     "apptabs/mobile/layout_bottom_icon.xml",
 		},
 		{
 			name:     "leading: tab with icon and text",
 			item:     container.NewTabItemWithIcon("Text1", theme.CancelIcon(), canvas.NewCircle(theme.BackgroundColor())),
 			location: container.TabLocationLeading,
-			want:     "apptabs/mobile/layout_bottom_icon_and_text.xml",
+			want:     "apptabs/mobile/layout_top_icon_and_text.xml",
 		},
 		{
 			name:     "leading: tab with text only",
 			item:     container.NewTabItem("Text2", canvas.NewCircle(theme.BackgroundColor())),
 			location: container.TabLocationLeading,
-			want:     "apptabs/mobile/layout_bottom_text.xml",
+			want:     "apptabs/mobile/layout_top_text.xml",
 		},
 		{
 			name:     "leading: tab with icon only",
 			item:     container.NewTabItemWithIcon("", theme.InfoIcon(), canvas.NewCircle(theme.BackgroundColor())),
 			location: container.TabLocationLeading,
-			want:     "apptabs/mobile/layout_bottom_icon.xml",
+			want:     "apptabs/mobile/layout_top_icon.xml",
 		},
 		{
 			name:     "trailing: tab with icon and text",
@@ -301,7 +301,7 @@ func TestAppTabs_SetTabLocation(t *testing.T) {
 
 	tabs.SetTabLocation(container.TabLocationLeading)
 	w.Resize(tabs.MinSize())
-	test.AssertRendersToMarkup(t, "apptabs/mobile/tab_location_bottom.xml", c, "leading is the same as bottom on mobile")
+	test.AssertRendersToMarkup(t, "apptabs/mobile/tab_location_top.xml", c, "leading is the same as top on mobile")
 
 	tabs.SetTabLocation(container.TabLocationBottom)
 	w.Resize(tabs.MinSize())

diff --git a/container/doctabs.go b/container/doctabs.go
@@ -145,7 +145,7 @@ func (t *DocTabs) SetItems(items []*TabItem) {
 
 // SetTabLocation sets the location of the tab bar
 func (t *DocTabs) SetTabLocation(l TabLocation) {
-	t.location = l
+	t.location = tabsAdjustedLocation(l)
 	t.Refresh()
 }
 
@@ -286,7 +286,11 @@ func (r *docTabsRenderer) buildTabButtons(count int) *fyne.Container {
 		if cells == 0 {
 			cells = 1
 		}
-		buttons.Layout = layout.NewGridLayout(cells)
+		if r.docTabs.location == TabLocationTop || r.docTabs.location == TabLocationBottom {
+			buttons.Layout = layout.NewGridLayoutWithColumns(cells)
+		} else {
+			buttons.Layout = layout.NewGridLayoutWithRows(cells)
+		}
 		iconPos = buttonIconTop
 	} else if r.docTabs.location == TabLocationLeading || r.docTabs.location == TabLocationTrailing {
 		buttons.Layout = layout.NewVBoxLayout()

diff --git a/container/doctabs_mobile_test.go b/container/doctabs_mobile_test.go
@@ -179,8 +179,17 @@ func TestDocTabs_HoverButtons(t *testing.T) {
 	test.MoveMouse(c, fyne.NewPos(75, 10))
 	test.AssertRendersToMarkup(t, "doctabs/mobile/hover_none.xml", c, "no hovering on mobile")
 
+	test.MoveMouse(c, fyne.NewPos(90, 10))
+	test.AssertRendersToMarkup(t, "doctabs/mobile/hover_none.xml", c, "no hovering on mobile")
+
 	test.MoveMouse(c, fyne.NewPos(10, 10))
 	test.AssertRendersToMarkup(t, "doctabs/mobile/hover_none.xml", c, "no hovering on mobile")
+
+	test.MoveMouse(c, fyne.NewPos(136, 10))
+	test.AssertRendersToMarkup(t, "doctabs/mobile/hover_none.xml", c, "no hovering on mobile")
+
+	test.MoveMouse(c, fyne.NewPos(104, 10))
+	test.AssertRendersToMarkup(t, "doctabs/mobile/hover_none.xml", c, "no hovering on mobile")
 }
 
 func TestDocTabs_Layout(t *testing.T) {
@@ -300,19 +309,17 @@ func TestDocTabs_SetTabLocation(t *testing.T) {
 	w.Resize(tabs.MinSize())
 	test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_top.xml", c)
 
-	// TODO: doc tabs support leading/trailing but rendering is currently broken (see #1962)
-	// tabs.SetTabLocation(container.TabLocationLeading)
-	// w.Resize(tabs.MinSize())
-	// test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_bottom.xml", c, "leading is the same as bottom on mobile")
+	tabs.SetTabLocation(container.TabLocationLeading)
+	w.Resize(tabs.MinSize())
+	test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_top.xml", c)
 
 	tabs.SetTabLocation(container.TabLocationBottom)
 	w.Resize(tabs.MinSize())
 	test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_bottom.xml", c)
 
-	// TODO: doc tabs support leading/trailing but rendering is currently broken (see #1962)
-	// tabs.SetTabLocation(container.TabLocationTrailing)
-	// w.Resize(tabs.MinSize())
-	// test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_bottom.xml", c, "trailing is the same as bottom on mobile")
+	tabs.SetTabLocation(container.TabLocationTrailing)
+	w.Resize(tabs.MinSize())
+	test.AssertRendersToMarkup(t, "doctabs/mobile/tab_location_bottom.xml", c)
 
 	tabs.SetTabLocation(container.TabLocationTop)
 	w.Resize(tabs.MinSize())
@@ -327,6 +334,9 @@ func TestDocTabs_Tapped(t *testing.T) {
 	item2 := &container.TabItem{Text: "Test2", Content: widget.NewLabel("Text 2")}
 	item3 := &container.TabItem{Text: "Test3", Content: widget.NewLabel("Text 3")}
 	tabs := container.NewDocTabs(item1, item2, item3)
+	tabs.CreateTab = func() *container.TabItem {
+		return &container.TabItem{Text: "Another", Content: widget.NewLabel("Another Tab")}
+	}
 	w := test.NewWindow(tabs)
 	defer w.Close()
 	w.SetPadded(false)
@@ -347,4 +357,11 @@ func TestDocTabs_Tapped(t *testing.T) {
 	test.TapCanvas(c, fyne.NewPos(10, 10))
 	require.Equal(t, 0, tabs.SelectedIndex())
 	test.AssertRendersToMarkup(t, "doctabs/mobile/tapped_first_selected.xml", c)
+
+	test.TapCanvas(c, fyne.NewPos(254, 10))
+	require.Equal(t, 3, tabs.SelectedIndex())
+	test.AssertRendersToMarkup(t, "doctabs/mobile/tapped_create_tab.xml", c)
+
+	test.TapCanvas(c, fyne.NewPos(286, 10))
+	test.AssertRendersToMarkup(t, "doctabs/mobile/tapped_all_tabs.xml", c)
 }
diff --git a/container/tabs.go b/container/tabs.go
@@ -61,6 +61,27 @@ type baseTabs interface {
 	tabLocation() TabLocation
 }
 
+func tabsAdjustedLocation(l TabLocation) TabLocation {
+	// Mobile has limited screen space, so don't put app tab bar on long edges
+	if d := fyne.CurrentDevice(); d.IsMobile() {
+		if o := d.Orientation(); fyne.IsVertical(o) {
+			if l == TabLocationLeading {
+				return TabLocationTop
+			} else if l == TabLocationTrailing {
+				return TabLocationBottom
+			}
+		} else {
+			if l == TabLocationTop {
+				return TabLocationLeading
+			} else if l == TabLocationBottom {
+				return TabLocationTrailing
+			}
+		}
+	}
+
+	return l
+}
+
 func buildPopUpMenu(t baseTabs, button *widget.Button, items []*fyne.MenuItem) *widget.PopUpMenu {
 	d := fyne.CurrentApp().Driver()
 	c := d.CanvasForObject(button)

diff --git a/container/testdata/apptabs/mobile/layout_bottom_ico.xml b/container/testdata/apptabs/mobile/layout_bottom_ico.xml
diff --git a/container/testdata/doctabs/mobile/tapped_all_tabs.xml b/container/testdata/doctabs/mobile/tapped_all_tabs.xml
@@ -0,0 +1,96 @@
+<canvas size="300x100">
+	<content>
+		<widget size="300x100" type="*container.DocTabs">
+			<container size="300x32">
+				<widget size="236x32" type="*widget.Scroll">
+					<container pos="-156,0" size="392x32">
+						<widget size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test1</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="99,0" size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test2</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="198,0" size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test3</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="297,0" size="95x32" type="*container.tabButton">
+							<text bold color="primary" pos="4,4" size="87x21">Another</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize" themed="primary"/>
+							</widget>
+						</widget>
+					</container>
+				</widget>
+				<container pos="240,0" size="60x32">
+					<widget size="28x32" type="*widget.Button">
+						<rectangle fillColor="button" size="28x32"/>
+						<rectangle fillColor="rgba(0,0,0,0)" pos="0,2" size="24x28"/>
+						<image fillMode="contain" pos="4,6" rsc="contentAddIcon" size="iconInlineSize"/>
+					</widget>
+					<widget pos="32,0" size="28x32" type="*widget.Button">
+						<rectangle fillColor="button" size="28x32"/>
+						<rectangle fillColor="rgba(0,0,0,0)" pos="0,2" size="24x28"/>
+						<image fillMode="contain" pos="4,6" rsc="more-horizontal.svg" size="iconInlineSize" themed="default"/>
+					</widget>
+				</container>
+			</container>
+			<rectangle fillColor="shadow" pos="0,32" size="300x4"/>
+			<rectangle fillColor="primary" pos="141,32" size="95x4"/>
+			<widget pos="0,36" size="300x64" type="*widget.Label">
+				<text pos="8,8" size="284x21">Another Tab</text>
+			</widget>
+		</widget>
+	</content>
+	<overlay>
+		<widget size="300x100" type="*widget.OverlayContainer">
+			<widget pos="226,0" size="74x100" type="*widget.Menu">
+				<widget size="74x100" type="*widget.Shadow">
+					<radialGradient centerOffset="0.5,0.5" pos="-4,-4" size="4x4" startColor="shadow"/>
+					<linearGradient endColor="shadow" pos="0,-4" size="74x4"/>
+					<radialGradient centerOffset="-0.5,0.5" pos="74,-4" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" pos="74,0" size="4x100" startColor="shadow"/>
+					<radialGradient centerOffset="-0.5,-0.5" pos="74,100" size="4x4" startColor="shadow"/>
+					<linearGradient pos="0,100" size="74x4" startColor="shadow"/>
+					<radialGradient centerOffset="0.5,-0.5" pos="-4,100" size="4x4" startColor="shadow"/>
+					<linearGradient angle="270" endColor="shadow" pos="-4,0" size="4x100"/>
+				</widget>
+				<widget size="74x100" type="*widget.Scroll">
+					<widget size="74x136" type="*widget.menuBox">
+						<rectangle fillColor="background" size="74x144"/>
+						<container pos="0,4" size="74x144">
+							<widget size="74x29" type="*widget.menuItem">
+								<text pos="8,4" size="39x21">Test1</text>
+							</widget>
+							<widget pos="0,33" size="74x29" type="*widget.menuItem">
+								<text pos="8,4" size="39x21">Test2</text>
+							</widget>
+							<widget pos="0,66" size="74x29" type="*widget.menuItem">
+								<text pos="8,4" size="39x21">Test3</text>
+							</widget>
+							<widget pos="0,99" size="74x29" type="*widget.menuItem">
+								<text pos="8,4" size="58x21">Another</text>
+							</widget>
+						</container>
+					</widget>
+					<widget pos="68,0" size="6x100" type="*widget.scrollBarArea">
+						<widget pos="3,0" size="3x73" type="*widget.scrollBar">
+							<rectangle fillColor="scrollbar" size="3x73"/>
+						</widget>
+					</widget>
+					<widget pos="0,100" size="74x0" type="*widget.Shadow">
+						<linearGradient endColor="shadow" pos="0,-8" size="74x8"/>
+					</widget>
+				</widget>
+			</widget>
+		</widget>
+	</overlay>
+</canvas>
diff --git a/container/testdata/doctabs/mobile/tapped_create_tab.xml b/container/testdata/doctabs/mobile/tapped_create_tab.xml
@@ -0,0 +1,53 @@
+<canvas size="300x100">
+	<content>
+		<widget size="300x100" type="*container.DocTabs">
+			<container size="300x32">
+				<widget size="236x32" type="*widget.Scroll">
+					<container pos="-156,0" size="392x32">
+						<widget size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test1</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="99,0" size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test2</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="198,0" size="95x32" type="*container.tabButton">
+							<text bold pos="4,4" size="87x21">Test3</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize"/>
+							</widget>
+						</widget>
+						<widget pos="297,0" size="95x32" type="*container.tabButton">
+							<text bold color="primary" pos="4,4" size="87x21">Another</text>
+							<widget pos="71,6" size="20x20" type="*container.tabCloseButton">
+								<image rsc="cancelIcon" size="iconInlineSize" themed="primary"/>
+							</widget>
+						</widget>
+					</container>
+				</widget>
+				<container pos="240,0" size="60x32">
+					<widget size="28x32" type="*widget.Button">
+						<rectangle fillColor="button" size="28x32"/>
+						<rectangle fillColor="rgba(0,0,0,0)" pos="0,2" size="24x28"/>
+						<image fillMode="contain" pos="4,6" rsc="contentAddIcon" size="iconInlineSize"/>
+					</widget>
+					<widget pos="32,0" size="28x32" type="*widget.Button">
+						<rectangle fillColor="button" size="28x32"/>
+						<rectangle size="0x0"/>
+						<image fillMode="contain" pos="4,6" rsc="more-horizontal.svg" size="iconInlineSize" themed="default"/>
+					</widget>
+				</container>
+			</container>
+			<rectangle fillColor="shadow" pos="0,32" size="300x4"/>
+			<rectangle fillColor="primary" pos="141,32" size="95x4"/>
+			<widget pos="0,36" size="300x64" type="*widget.Label">
+				<text pos="8,8" size="284x21">Another Tab</text>
+			</widget>
+		</widget>
+	</content>
+</canvas>