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

Bug 1134282 - Make home panel tab icons scrollable #154

Closed

Conversation

sleroux
Copy link

@sleroux sleroux commented Feb 14, 2015

Hey!

I noticed this issue on Bugzilla (https://bugzilla.mozilla.org/show_bug.cgi?id=1131284) and figured I would give it a try. From the wording it seemed like you just wanted the home button panel to be scrollable in case there are more buttons in the future so I wrapped your existing toolbar container in a scroll view. Do you also see the user being able to swipe left/right on the view controllers below to switch between tabs (similar to the Android ViewPager control)?

@thebnich
Copy link
Contributor

Thanks for the PR! I made a build using this and added some extra buttons here so that the toolbar overflowed, but I'm not able to scroll horizontally. Is that what this PR is for, or something else?

@sleroux
Copy link
Author

sleroux commented Feb 18, 2015

Ah yup I see the problem. I was doing a test by changing the content size to simulate a larger panel. Let me fix this up.

@sleroux
Copy link
Author

sleroux commented Feb 18, 2015

I notice that the buttons are getting laid out manually in the container's layoutSubviews call here: https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/HomePanelViewController.swift#L245 which is making the container frame not update it's bounds. Was this to center the buttons? With additional buttons being added, do we need this code anymore? If not I can replace it with autolayout to make the items equally spaced out.

@thebnich
Copy link
Contributor

The specs for bug 1131284 have changed quite a bit since it was filed, and scrolling is no longer necessary for the V1 release. That said, it would still be useful to have for the future since you already have an open PR for it. I moved this scrolling to a separate bug (bug 1134282).

@thebnich thebnich changed the title Bug 1131284 - Create pager UI for home panels Bug 1134282 - Make home panel tab icons scrollable Feb 18, 2015
@thebnich
Copy link
Contributor

I notice that the buttons are getting laid out manually in the container's layoutSubviews call here

Yeah, this file is one of the older artifacts from our very early prototype implementation (before deciding that we were even making a web browser!). It doesn't work well at all across different devices; on my smaller iPod touch, the icons don't even fit on the screen. I have a mostly complete patch I'll put up today that refactors most of this code, including autolayout :)

@sleroux
Copy link
Author

sleroux commented Feb 19, 2015

Alright sounds good! Feel free to close this PR if you want - I can hunt something else down 👍

@thebnich
Copy link
Contributor

That's probably the easiest way forward since the spec changed for V1, but thanks for the contribution! Here's a full list of unassigned bugs if you want to find something else :)

@thebnich thebnich closed this Feb 19, 2015
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.

2 participants