Aalborg site menu does not work in mobile firefox #9110

Closed
beck24 opened this Issue Nov 5, 2015 · 16 comments

Comments

Projects
None yet
5 participants
@beck24
Member

beck24 commented Nov 5, 2015

No description provided.

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 5, 2015

Contributor

Which OS (Android, iOS or FFOS)? Version of Firefox? I can only test on FFOS and it works for me. On iOS it might be a browser issue as Firefox for iphones has just been released if I remember correctly.

Contributor

iionly commented Nov 5, 2015

Which OS (Android, iOS or FFOS)? Version of Firefox? I can only test on FFOS and it works for me. On iOS it might be a browser issue as Firefox for iphones has just been released if I remember correctly.

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 Nov 5, 2015

Member

Android - it's not working on a clients 1.12 installation and it's also not working on the community. Leads me to think this is a change in FF as I'm pretty sure it was working in the past.

FF 42.0

Member

beck24 commented Nov 5, 2015

Android - it's not working on a clients 1.12 installation and it's also not working on the community. Leads me to think this is a change in FF as I'm pretty sure it was working in the past.

FF 42.0

@mrclay mrclay added bug and removed bc break labels Nov 5, 2015

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 5, 2015

Contributor

I was able to reproduce - doesn't work even when using FF on an Android emulator (!)... The problem seems to be the collapsed mobile site menu. When requesting the desktop version of the site the menu items can be clicked without issues.

Contributor

iionly commented Nov 5, 2015

I was able to reproduce - doesn't work even when using FF on an Android emulator (!)... The problem seems to be the collapsed mobile site menu. When requesting the desktop version of the site the menu items can be clicked without issues.

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 5, 2015

Contributor

Could it be that FF on android has some issues with the "empty" span elements within mod/aalborg_theme/views/default/page/elements/navbar.php? If I add some text output like

<span class="icon-bar">Text</span>

the menu open/closes alright. But I have no idea how to get it working without adding some visible output unfortunately.

Contributor

iionly commented Nov 5, 2015

Could it be that FF on android has some issues with the "empty" span elements within mod/aalborg_theme/views/default/page/elements/navbar.php? If I add some text output like

<span class="icon-bar">Text</span>

the menu open/closes alright. But I have no idea how to get it working without adding some visible output unfortunately.

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 5, 2015

Contributor

Maybe solution for 2.0 would be usage of fontawesome navicon (fa-bars). Can't test though (only have an 1.10 installation that I can use with the android emulator).

Contributor

iionly commented Nov 5, 2015

Maybe solution for 2.0 would be usage of fontawesome navicon (fa-bars). Can't test though (only have an 1.10 installation that I can use with the android emulator).

@topdog08

This comment has been minimized.

Show comment
Hide comment
@topdog08

topdog08 Nov 5, 2015

What about <span class="icon-bar">&nbsp;</span>?

topdog08 commented Nov 5, 2015

What about <span class="icon-bar">&nbsp;</span>?

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 8, 2015

Contributor

I had already tries using non-breaking spaces but while this resulted in the menu toggle to work it also disturbed the output.

Contributor

iionly commented Nov 8, 2015

I had already tries using non-breaking spaces but while this resulted in the menu toggle to work it also disturbed the output.

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 Nov 8, 2015

Member

I fixed it on the client site by replacing the icon with an image instead of markup, but markup is ultimately the better solution if browsers can stop being dumb.

Member

beck24 commented Nov 8, 2015

I fixed it on the client site by replacing the icon with an image instead of markup, but markup is ultimately the better solution if browsers can stop being dumb.

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 8, 2015

Contributor

I don't see any other fix than using an image for Elgg 1.X either. For Elgg 2.X switching to a fontawesome icon should work (and there's even the fa-bars icon that would exactly fit).

Contributor

iionly commented Nov 8, 2015

I don't see any other fix than using an image for Elgg 1.X either. For Elgg 2.X switching to a fontawesome icon should work (and there's even the fa-bars icon that would exactly fit).

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Nov 8, 2015

Contributor

Would setting the css display to block or inline block on an icon fix the issue?

Contributor

hypeJunction commented Nov 8, 2015

Would setting the css display to block or inline block on an icon fix the issue?

@iionly

This comment has been minimized.

Show comment
Hide comment
@iionly

iionly Nov 8, 2015

Contributor

Neither inline nor inline-blocks works for me.

Contributor

iionly commented Nov 8, 2015

Neither inline nor inline-blocks works for me.

@mrclay mrclay added the critical label Nov 29, 2015

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Nov 29, 2015

Member

Sounds like 1.12 needs an image, and 2.0 will use fa-bars.

Member

mrclay commented Nov 29, 2015

Sounds like 1.12 needs an image, and 2.0 will use fa-bars.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Nov 30, 2015

Member

Change for 1.12: #9171

Member

mrclay commented Nov 30, 2015

Change for 1.12: #9171

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Nov 30, 2015

Member

I've started a discussion around potential action on this.

Member

mrclay commented Nov 30, 2015

I've started a discussion around potential action on this.

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Dec 14, 2015

fix(a11y): aalborg mobile site menu uses the Font Awesome fa-bars icon
BREAKING CHANGE:
In aalborg_theme, the view `page/elements/navbar` now uses an icon for the
mobile menu selector (formerly an image). The `bars.png` image and supporting
CSS for the 1.12 rendering has been removed.

Fixes #9110
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Dec 14, 2015

Member

PR for 2.0: #9226. This is 1 of 2 final blockers for 2.0.

Member

mrclay commented Dec 14, 2015

PR for 2.0: #9226. This is 1 of 2 final blockers for 2.0.

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Dec 14, 2015

fix(a11y): aalborg mobile site menu uses the Font Awesome fa-bars icon
BREAKING CHANGE:
In aalborg_theme, the view `page/elements/navbar` now uses an icon for the
mobile menu selector (formerly an image). The `bars.png` image and supporting
CSS for the 1.12 rendering has been removed.

Fixes #9110

@mrclay mrclay closed this in e96f079 Dec 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment