Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-38347

Left and right arrows for calendar not showing up on Android

    Details

    • Testing Instructions:
      Hide

      In all supported browsers view a page with a calendar block and check the icons for the next/previous month are shown correctly.
      Ensure you test under Windows, Linux and OSX.

      Show
      In all supported browsers view a page with a calendar block and check the icons for the next/previous month are shown correctly. Ensure you test under Windows, Linux and OSX.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-38347-master
    • Sprint:
      FRONTEND Sprint 1
    • Story Points (Obsolete):
      3
    • Sprint:
      FRONTEND Sprint 1

      Description

      The default left and right triangle icons (as used in the calender block to move to the previous or next month and as a seperator in the breadcrumb trail) have issues on Android.

      On Android 4.0 up, the font is called Roboto. It seems to not have the left pointing version.

      I've not seen it myself but I've had reports that 2.3 devices (which use a font called Droid Sans) are missing both versions.

      Here's a list of the geometric shapes available in Unicode:

      http://www.alanwood.net/unicode/geometric_shapes.html

      We currently use:

      25BA BLACK RIGHT-POINTING POINTER
      25C4 BLACK LEFT-POINTING POINTER

      I believe these particular shapes may have been originally chosen to ensure they displayed on IE6/XP (IE6 has issues where it didn't fall back if the first chosen font didn't contain the character, proper browsers continue to search through all your fonts).

      Perhaps a different triangle shape could be used? Though testing that it works on everything else could be a pain.

      Longer term these could be provided by an icon font or SVG icons.

      I'm marking this as RTL-related as I assume the left facing one is used more in those situations, and at least on Android 4.0 that seems to be the main issue.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            bawjaws David Scotson added a comment -

            The code for which arrows we currently use is actually more complicated than I thought, it sends different arrows to different user agents, so could be tweaked specifically for android:

            https://github.com/moodle/moodle/blob/master/lib/outputlib.php#L485-L522

            Show
            bawjaws David Scotson added a comment - The code for which arrows we currently use is actually more complicated than I thought, it sends different arrows to different user agents, so could be tweaked specifically for android: https://github.com/moodle/moodle/blob/master/lib/outputlib.php#L485-L522
            Hide
            bawjaws David Scotson added a comment -

            The Font Awesome stuff I've done for another bug gives, if used, another way to fix this, since it provides multiple sets of direction glyhps:

            http://fortawesome.github.io/Font-Awesome/#icons-directional

            Just needs you to override the larrow and rarrow renderers.

            Should work on Android 2.2+

            Show
            bawjaws David Scotson added a comment - The Font Awesome stuff I've done for another bug gives, if used, another way to fix this, since it provides multiple sets of direction glyhps: http://fortawesome.github.io/Font-Awesome/#icons-directional Just needs you to override the larrow and rarrow renderers. Should work on Android 2.2+
            Hide
            salvetore Michael de Raadt added a comment -

            Is this specifically a problem with the Bootstrap theme? It is recorded as affecting 2.4.1 but it is in the Bootstrap meta.

            Show
            salvetore Michael de Raadt added a comment - Is this specifically a problem with the Bootstrap theme? It is recorded as affecting 2.4.1 but it is in the Bootstrap meta.
            Hide
            phalacee Jason Fowler added a comment -

            I've only looked at it briefly, so I am not 100% sure. I don't think it is a deal breaker, but I was working on it before getting asked to build the Active Directory test server.

            Show
            phalacee Jason Fowler added a comment - I've only looked at it briefly, so I am not 100% sure. I don't think it is a deal breaker, but I was working on it before getting asked to build the Active Directory test server.
            Hide
            bawjaws David Scotson added a comment -

            No it affects all themes and versions currently.

            There was some possibility of working around it in the Bootstrap theme (via theme settings, renderers or CSS) and mobile/responsive was a focus for Bootstrap/Clean so that's when I spotted the problem. But the check_theme_arrows function that I linked to above in outputlib is probably the best place to fix it for all themes at once.

            Show
            bawjaws David Scotson added a comment - No it affects all themes and versions currently. There was some possibility of working around it in the Bootstrap theme (via theme settings, renderers or CSS) and mobile/responsive was a focus for Bootstrap/Clean so that's when I spotted the problem. But the check_theme_arrows function that I linked to above in outputlib is probably the best place to fix it for all themes at once.
            Hide
            salvetore Michael de Raadt added a comment -

            Thanks, guys.

            I've taken this off the Bootstrap meta.

            Show
            salvetore Michael de Raadt added a comment - Thanks, guys. I've taken this off the Bootstrap meta.
            Hide
            phalacee Jason Fowler added a comment -

            Should be backportable. Once peer review has been completed, I will create clean branches for other versions.

            Show
            phalacee Jason Fowler added a comment - Should be backportable. Once peer review has been completed, I will create clean branches for other versions.
            Hide
            phalacee Jason Fowler added a comment -

            Branches created ... cherry picks cleanly anyway ...

            Show
            phalacee Jason Fowler added a comment - Branches created ... cherry picks cleanly anyway ...
            Hide
            damyon Damyon Wiese added a comment -

            Note: I haven't got access from here (working from home today) to verify that this now works on android - so that will be upto the testers.

            The change looks fine to me - we could come up with some alternative for this like an image with a data-url - but IMO the benefit would not be worth the effort so I agree with this fix.

            [Y] Syntax
            [-] Output
            [Y] Whitespace
            [-] Language
            [-] Databases
            [Y] Testing
            [-] Security
            [-] Documentation
            [?] Git - It's not clear from the comments who wrote the patch - either way it would be best to give David some credit in the git commit for the detailed report.
            [Y] Sanity check

            The patch is fine - feel free to send for integration (if you need to give David credit as noted above, please do this first).

            Show
            damyon Damyon Wiese added a comment - Note: I haven't got access from here (working from home today) to verify that this now works on android - so that will be upto the testers. The change looks fine to me - we could come up with some alternative for this like an image with a data-url - but IMO the benefit would not be worth the effort so I agree with this fix. [Y] Syntax [-] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [?] Git - It's not clear from the comments who wrote the patch - either way it would be best to give David some credit in the git commit for the detailed report. [Y] Sanity check The patch is fine - feel free to send for integration (if you need to give David credit as noted above, please do this first).
            Hide
            phalacee Jason Fowler added a comment -

            The patch was my work, although David was kind enough to point out the location the patch should go in a link to the git repository.

            Pushing for integration now.

            Show
            phalacee Jason Fowler added a comment - The patch was my work, although David was kind enough to point out the location the patch should go in a link to the git repository. Pushing for integration now.
            Hide
            poltawski Dan Poltawski added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jason, this has been integrated now.

            Just noting I had to fix whitespace during integration.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jason, this has been integrated now. Just noting I had to fix whitespace during integration.
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.3, 2.4, 2.5 and master.

            I tested this with the following OS-browser combinations.

            Windows: FF, Chrome, IE10
            Linux: FF, Chrome
            OSX: Safari, FF, Chrome
            Android 4: Stock, Chrome
            Android 2.3: Stock
            IOS: Safari

            In Android, the forward/back icons appear as arrows instead of triangles; this is fine. I noted that the back arrow was missing in Android 4 Stock when viewing the page in "Desktop" mode, but I don't think that's a reportable bug.

            I reported a very trivial issue with sizing of the forward/back icons under FF on OSX.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.3, 2.4, 2.5 and master. I tested this with the following OS-browser combinations. Windows: FF, Chrome, IE10 Linux: FF, Chrome OSX: Safari, FF, Chrome Android 4: Stock, Chrome Android 2.3: Stock IOS: Safari In Android, the forward/back icons appear as arrows instead of triangles; this is fine. I noted that the back arrow was missing in Android 4 Stock when viewing the page in "Desktop" mode, but I don't think that's a reportable bug. I reported a very trivial issue with sizing of the forward/back icons under FF on OSX.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Thanks for giving me joys and smiles
            Thanks for sharing my trouble's pile

            Thanks for wipeing the tears of my eye
            Thanks for showing me the glad view of sky

            Thanks for lending me your shoulders to lean
            Thanks for giving my words a proper mean

            Thanks for telling me the value of life
            Thanks for showing me the rules to survive

            Thanks for lending me the sympathetic ears
            Thanks for showing how much you care

            From all this what I mean in the end
            Is thanks for being my special friend.

            – Seema Chowdhury

            Sent upstream so... closing, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for giving me joys and smiles Thanks for sharing my trouble's pile Thanks for wipeing the tears of my eye Thanks for showing me the glad view of sky Thanks for lending me your shoulders to lean Thanks for giving my words a proper mean Thanks for telling me the value of life Thanks for showing me the rules to survive Thanks for lending me the sympathetic ears Thanks for showing how much you care From all this what I mean in the end Is thanks for being my special friend. – Seema Chowdhury Sent upstream so... closing, thanks!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13

                  Agile