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

[Ticket/14194] Responsive Quick Links Menu Broken #3931

Merged
merged 4 commits into from Oct 11, 2015
Merged

[Ticket/14194] Responsive Quick Links Menu Broken #3931

merged 4 commits into from Oct 11, 2015

Conversation

hanakin
Copy link
Member

@hanakin hanakin commented Sep 27, 2015

@hanakin
Copy link
Member Author

hanakin commented Sep 27, 2015

@VSEphpbb please review when u get a chance

@iMattPro
Copy link
Member

2 Issues I see:

  1. It is still supposed to say Quick Links next to the hamburger in responsive mode.
  2. Legacy "small-icon" object positioning is not handled correctly anymore.

The result of this PR (see the Test item):
screen shot 2015-09-27 at 10 19 14 am

The current 3.1.x implementation:
screen shot 2015-09-27 at 10 19 28 am

@hanakin
Copy link
Member Author

hanakin commented Sep 27, 2015

quicklinks text is removed at responsive
screenshot 2015-09-27 19 31 12
screenshot 2015-09-27 19 31 28

as for small icons not sure on that its going to require some research as it was not really supported just included back in for bc.

@iMattPro
Copy link
Member

Well, as for the quick links, as you can see, the Quick Links text is vanishing before it is supposed to. The hamburger should not be left hanging out there all by itself with no label when there is still plenty of room for one...if at all possible ;)

3.1.x:
screen shot 2015-09-27 at 11 02 47 am

This PR:
screen shot 2015-09-27 at 10 58 34 am

@hanakin
Copy link
Member Author

hanakin commented Sep 29, 2015

@VSEphpbb small icon should be fixed, not sure what exactly is want for the Quicklinks I made an adjustment so let me know

@iMattPro
Copy link
Member

Still some responsive issues. They may not be related to this PR specifically, but they're what I find when looking so I'm just putting the info up here.

First, I noticed this major blowout in small responsive mode to the forum header bar (it's testable on area51 right now):
screen shot 2015-09-28 at 9 58 10 pm

Next. The padding around the post button and search bars was better in 3.1 vs 3.2:
3.1.x
screen shot 2015-09-28 at 10 00 06 pm

3.2.x
screen shot 2015-09-28 at 10 00 11 pm

@hanakin
Copy link
Member Author

hanakin commented Sep 29, 2015

the second one is any easy fix in 3.1 a global margin was applied to the right side which i removed as they were not aligned i will add it back responsively. As for the first one I seen that not sure its related to this PR but its a dousey I have not been able to track down the source of the issue as I believe its steaming from a combination of js buffoonery and responsive css making it extremely difficult to narrow down as JS is not really debuggable for this type of thing which is why its never recommended for handle responsive rendering in this manner.

PS It will take me some time to get to this as I am pretty much out for the next two weeks

@CHItA CHItA added this to the 3.2.0-a1 milestone Sep 29, 2015
@hanakin
Copy link
Member Author

hanakin commented Sep 30, 2015

viewforum header fix: #3942

@hanakin
Copy link
Member Author

hanakin commented Oct 1, 2015

ready for review

@marc1706
Copy link
Member

marc1706 commented Oct 2, 2015

What are the FAQ, ACP, and MCP links supposed to do on very small screens?
This is what it supposedly looks like on an iPhone 4:
yourdomain com - index page
The notifications icon on the next line kinda bugs me.

@hanakin
Copy link
Member Author

hanakin commented Oct 3, 2015

is this on area51 or with this merge? it may have gotten messed up when i rebased will look at it when i get a chance...

@marc1706
Copy link
Member

marc1706 commented Oct 5, 2015

That's on up-to-date master with this PR merged.

@Nicofuma
Copy link
Member

Nicofuma commented Oct 5, 2015

by the way a51 is now sync with master

@hanakin
Copy link
Member Author

hanakin commented Oct 8, 2015

@marc1706 im not seeing what ur seeing
screenshot 2015-10-08 08 17 30

@marc1706
Copy link
Member

marc1706 commented Oct 9, 2015

@hanakin could you rebase this?

@marc1706
Copy link
Member

Looks good on an emulated iPhone 4 now, too. Thx

marc1706 added a commit to marc1706/phpbb that referenced this pull request Oct 11, 2015
[Ticket/14194] Responsive Quick Links Menu Broken
@marc1706 marc1706 merged commit 8acbb20 into phpbb:master Oct 11, 2015
@hanakin hanakin deleted the ticket/14194 branch May 17, 2017 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants