Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tab button layout on mobile #1962

Closed
wants to merge 4 commits into from

Conversation

stuartmscott
Copy link
Member

@stuartmscott stuartmscott commented Feb 15, 2021

Description:

Tab buttons were always rendered in a grid of columns, this PR uses a grid of rows when the tab location is leading or trailing

Checklist:

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

Copy link
Member

@andydotxyz andydotxyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but not quite the complete solution - 2 cases seem wrong.

Mobile, Portrait, Leading - tabs appear at bottom (should be top)
Mobile, Landscape, Bottom - tabs appear at left (leading), but should be right

@stuartmscott
Copy link
Member Author

Where are you getting these expectations from?

From the ticket (#1310), AppTabs is a combination of two Material Design components;

  • Bottom Navigation - which, as the name, suggests doesn't appear at the top.
  • Navigation Rail - "The navigation rail is placed on the left side of the screen for left-to-right languages, and on the right side of the screen for right-to-left languages."

@andydotxyz
Copy link
Member

Where are you getting these expectations from?

From how it worked before?

@stuartmscott
Copy link
Member Author

From how it worked before?

I don't think that is true, Fyne v2.0.0 AppTab on Mobile always moved Leading & Trailing to Bottom (https://github.com/fyne-io/fyne/blob/master/container/apptabs.go#L323):

func (r *appTabsRenderer) adaptedLocation() TabLocation {
	tabLocation := r.container.tabLocation
	if fyne.CurrentDevice().IsMobile() && (tabLocation == TabLocationLeading || tabLocation == TabLocationTrailing) {
		return TabLocationBottom
	}

	return r.container.tabLocation
}

And does not have different behaviour for Mobile Portrait vs Mobile Landscape

@andydotxyz
Copy link
Member

Fyne v2.0.0 AppTab on Mobile always moved Leading & Trailing to Bottom

Strange, seems I misremembered how it was previously working.

@andydotxyz andydotxyz dismissed their stale review February 21, 2021 12:50

I misremembered how it used to work, needs re-testing

toaster added a commit to toaster/fyne that referenced this pull request Feb 26, 2021
This is not the expected state (see fyne-io#1962).
This helps to recognize the difference between the current state and the fixed layout.
@toaster toaster mentioned this pull request Feb 26, 2021
2 tasks
toaster added a commit to toaster/fyne that referenced this pull request Feb 28, 2021
This is not the expected state (see fyne-io#1962).
This helps to recognize the difference between the current state and the fixed layout.
@toaster toaster mentioned this pull request Feb 28, 2021
@stuartmscott stuartmscott deleted the tabs branch March 25, 2021 15:58
andydotxyz added a commit to andydotxyz/fyne that referenced this pull request Mar 25, 2021
Picked from Stuart's commit from PR fyne-io#1962
@andydotxyz andydotxyz mentioned this pull request Mar 25, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants