Moodle
  1. Moodle
  2. MDL-38347

Left and right arrows for calendar not showing up on Android

    Details

    • Story Points:
      3
    • Rank:
      48233
    • 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.

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Michael de Raadt added a comment -

          Thanks, guys.

          I've taken this off the Bootstrap meta.

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

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

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

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

          Show
          Jason Fowler added a comment - Branches created ... cherry picks cleanly anyway ...
          Hide
          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 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
          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
          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
          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
          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
          Sam Hemelryk added a comment -

          Thanks Jason, this has been integrated now.

          Just noting I had to fix whitespace during integration.

          Show
          Sam Hemelryk added a comment - Thanks Jason, this has been integrated now. Just noting I had to fix whitespace during integration.
          Hide
          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
          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
          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
          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:

                Agile