Moodle
  1. Moodle
  2. MDL-32848

move icon for section and blocks temporarily appears on page for course formats with d&d support

    Details

    • Affected Branches:
      MOODLE_23_STABLE
    • Rank:
      39902

      Description

      solution could be to either detect if course format can move these via d&d and use .jshidden for these icons, alternatively use it unconditionally and add JS that enables them later for course formants that do not support d&d (hopefully it will be a minority)

      I am going to bump up priority because it imho does not look very professional

        Issue Links

          Activity

          Hide
          Andrew Nicols added a comment -

          Adding Ruslan as a watcher.

          We need to add hiddenifjs to the moveup/movedown/move icons here.

          At present all course formats do indeed support the new JS (or will do RSN) but we still need to handle formats which don't support AJAX.
          The moveup/movedown icons are added by the renderer so we need to somehow pass the ajax enabled status to the renderer.

          For blocks, we need to only add the class on certain pages at present.

          Any thoughts Ruslan?

          Show
          Andrew Nicols added a comment - Adding Ruslan as a watcher. We need to add hiddenifjs to the moveup/movedown/move icons here. At present all course formats do indeed support the new JS (or will do RSN) but we still need to handle formats which don't support AJAX. The moveup/movedown icons are added by the renderer so we need to somehow pass the ajax enabled status to the renderer. For blocks, we need to only add the class on certain pages at present. Any thoughts Ruslan?
          Hide
          Ruslan Kabalin added a comment -

          I agree with Andrew, there might be themes that does not support dragdrop, to removing these buttons for them initially will be a shame.

          Yes, I had some thoughts about this. What is missed in Moodle, is the way to determine if JS is actually enabled/supported by browser. We use the code that degrades nicely, which is good, but not sufficient in the cases like above.

          The approach here could be to actually make browser informing server if it can handle JS. The flow of this process could be:

          1. On first Moodle page load. The renderer adds the js snippet that does ajax callback and refresh the page on success.
          2. The back-end which fetches ajax request, sets $SESSION->browserjsenabled = true;
          3. $SESSION->browserjsenabled can be used everywhere for checking that browser uses JS (e.g. in renderer for moveup/movedown buttons generation)
          4. If $SESSION->browserjsenabled has been set, renderer does not produce a callback snippet before session is expired again.

          Drawbacks:

          • Adds testing difficulties, turning JS off in the browser which informed Moodle that JS is enabled will not bring desired result. Moodle will still think that JS enabled. Hmm, actually this can be solved by having <noscript> with
            <meta http-equiv="refresh" content="0; URL=index.php?nojs">

            to reset $SESSION->browserjsenabled when browser stopped supporting JS all of a sudden.

          • May make people relying on this rather than producing a good degradable code.
          Show
          Ruslan Kabalin added a comment - I agree with Andrew, there might be themes that does not support dragdrop, to removing these buttons for them initially will be a shame. Yes, I had some thoughts about this. What is missed in Moodle, is the way to determine if JS is actually enabled/supported by browser. We use the code that degrades nicely, which is good, but not sufficient in the cases like above. The approach here could be to actually make browser informing server if it can handle JS. The flow of this process could be: On first Moodle page load. The renderer adds the js snippet that does ajax callback and refresh the page on success. The back-end which fetches ajax request, sets $SESSION->browserjsenabled = true; $SESSION->browserjsenabled can be used everywhere for checking that browser uses JS (e.g. in renderer for moveup/movedown buttons generation) If $SESSION->browserjsenabled has been set, renderer does not produce a callback snippet before session is expired again. Drawbacks: Adds testing difficulties, turning JS off in the browser which informed Moodle that JS is enabled will not bring desired result. Moodle will still think that JS enabled. Hmm, actually this can be solved by having <noscript> with <meta http-equiv= "refresh" content= "0; URL=index.php?nojs" > to reset $SESSION->browserjsenabled when browser stopped supporting JS all of a sudden. May make people relying on this rather than producing a good degradable code.
          Hide
          Ruslan Kabalin added a comment -

          Added Dan, as we discussed this approach in the past.

          Show
          Ruslan Kabalin added a comment - Added Dan, as we discussed this approach in the past.
          Hide
          Petr Škoda added a comment - - edited

          Wait, can we not determine in PHP if the icons are going to be needed? I guess we can ask the course format, theme, user prefs, browser type, etc. And only if we are not sure keep the current behaviour. I think that pages that jump around on load are ugly and annoying.

          Show
          Petr Škoda added a comment - - edited Wait, can we not determine in PHP if the icons are going to be needed? I guess we can ask the course format, theme, user prefs, browser type, etc. And only if we are not sure keep the current behaviour. I think that pages that jump around on load are ugly and annoying.
          Hide
          Dan Poltawski added a comment -

          Urm, I only created MDL-32837 today and you've already duplicated it?!

          Show
          Dan Poltawski added a comment - Urm, I only created MDL-32837 today and you've already duplicated it?!
          Hide
          Ruslan Kabalin added a comment - - edited

          True Petr, we can, similar to the way we do in include_course_ajax(), bit that will not tell us about dragdrop support. In fact, we can add $THEME->supportsdragdrop=true to make sure that author acknowledge that the theme is dragdrop-compatible. Though not sure if this is the best way.

          Show
          Ruslan Kabalin added a comment - - edited True Petr, we can, similar to the way we do in include_course_ajax(), bit that will not tell us about dragdrop support. In fact, we can add $THEME->supportsdragdrop=true to make sure that author acknowledge that the theme is dragdrop-compatible. Though not sure if this is the best way.
          Hide
          Dan Poltawski added a comment -

          Ruslan: I do not like that solution and I do not see how your solution helps here, we should hide the move icons from us the jshide class if they are not to be shown with JS off.

          Show
          Dan Poltawski added a comment - Ruslan: I do not like that solution and I do not see how your solution helps here, we should hide the move icons from us the jshide class if they are not to be shown with JS off.
          Hide
          Ruslan Kabalin added a comment -

          Urm, I only created MDL-32837 today and you've already duplicated it?!

          Then the issue is indeed important

          Show
          Ruslan Kabalin added a comment - Urm, I only created MDL-32837 today and you've already duplicated it?! Then the issue is indeed important
          Hide
          Ruslan Kabalin added a comment - - edited

          Ruslan: I do not like that solution and I do not see how your solution helps here, we should hide the move icons from us the jshide class if they are not to be shown with JS off

          Not sure if I got what you mean correctly, the main problem is that we can't guarantee that theme supports dragdrop, so removing icons could make sections and blocks dragging impossible (there will be no icons for old-style two-steps dragging). That is the reason why we can't simply add .jshidden

          Show
          Ruslan Kabalin added a comment - - edited Ruslan: I do not like that solution and I do not see how your solution helps here, we should hide the move icons from us the jshide class if they are not to be shown with JS off Not sure if I got what you mean correctly, the main problem is that we can't guarantee that theme supports dragdrop, so removing icons could make sections and blocks dragging impossible (there will be no icons for old-style two-steps dragging). That is the reason why we can't simply add .jshidden
          Hide
          Dan Poltawski added a comment -

          Well, how does knowing if JS is enabled at the point of html generation help with that?

          (The solution to that problem is to make theme declare support for dragdrop - in fact there is already a related setting to this for themes $THEME->courseajax)

          Show
          Dan Poltawski added a comment - Well, how does knowing if JS is enabled at the point of html generation help with that? (The solution to that problem is to make theme declare support for dragdrop - in fact there is already a related setting to this for themes $THEME->courseajax)
          Hide
          Ruslan Kabalin added a comment -

          Yep Dan, $THEME->supportsdragdrop=true is what suggested above as alternative.

          You are right, the solution may help not glimpsing icons, but will not help in case if theme does not support it, my underestimation :/

          Show
          Ruslan Kabalin added a comment - Yep Dan, $THEME->supportsdragdrop=true is what suggested above as alternative. You are right, the solution may help not glimpsing icons, but will not help in case if theme does not support it, my underestimation :/

            People

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

              Dates

              • Created:
                Updated: