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

      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

        Gliffy Diagrams

          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 Skoda 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 Skoda 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 :/
            Hide
            Andrew Nicols added a comment -

            Unassigning from myself as I won't be working on this in the near future.

            Show
            Andrew Nicols added a comment - Unassigning from myself as I won't be working on this in the near future.

              People

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

                Dates

                • Created:
                  Updated: