Moodle
  1. Moodle
  2. MDL-33056

Drag and drop upload status bar needs better placement and styling [#dndupload-status]

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: AJAX and JavaScript
    • Labels:
    • Rank:
      40236

      Description

      The drag and drop upload status bar that is displayed with course ajax enabled when editing a course needs to be better placed and probably could be re-styled.. although perhaps styling is a separate issue.

      At present the placement is the biggest problem, its not theme friendly and could possibly create problems for themes that have alternative layouts to the one we use presently.
      It represents a high chance of regression and should really be fixed before 2.3 release.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Increasing priority, marking must fix for 2.3, and marking triaged.

          Show
          Sam Hemelryk added a comment - Increasing priority, marking must fix for 2.3, and marking triaged.
          Hide
          Davo Smith added a comment -

          I'm very happy for suggestions about the placement - it was put there (and styled) as the best I could come up with as a temporary solution.

          The ID of the DOM element to add the message DIV to is defined clearly at the top of the javascript file and the styling is entirely within the base theme.

          Show
          Davo Smith added a comment - I'm very happy for suggestions about the placement - it was put there (and styled) as the best I could come up with as a temporary solution. The ID of the DOM element to add the message DIV to is defined clearly at the top of the javascript file and the styling is entirely within the base theme.
          Hide
          Martin Dougiamas added a comment -

          I'd float it "above" the page, right under the nav bar, and make it appear only for a few seconds directly after "turn editing on" is clicked. Then fade away like my youth (only faster).

          Show
          Martin Dougiamas added a comment - I'd float it "above" the page, right under the nav bar, and make it appear only for a few seconds directly after "turn editing on" is clicked. Then fade away like my youth (only faster).
          Hide
          Sam Marshall added a comment -

          Suggestions:

          (a) Add this at top of middle section of page, i.e. at least don't push the blocks down even if the main contents are pushed down. That way it also appears nearer where you are supposed to drag things.

          (b) A little (X) meaning don't-show-this-again (user pref) would solve the annoyance problem and be similar to interface used on other sites.

          It's also been suggested that it should appear on top transparent and then fade out - while that might be nice I think it will get annoying if it happens every page load so really, the don't-show-again is a better solution (but more work, so could be latered I guess).

          Note: there is already ajax code to allow set user preference. (Have to do stuff in php to get it, though, obviously...)

          Show
          Sam Marshall added a comment - Suggestions: (a) Add this at top of middle section of page, i.e. at least don't push the blocks down even if the main contents are pushed down. That way it also appears nearer where you are supposed to drag things. (b) A little (X) meaning don't-show-this-again (user pref) would solve the annoyance problem and be similar to interface used on other sites. It's also been suggested that it should appear on top transparent and then fade out - while that might be nice I think it will get annoying if it happens every page load so really, the don't-show-again is a better solution (but more work, so could be latered I guess). Note: there is already ajax code to allow set user preference. (Have to do stuff in php to get it, though, obviously...)
          Hide
          Dan Poltawski added a comment -

          I've implemented both of Sam (M)'s suggestions.

          Problem with this approach is that its hard to get the message back, its not clear that you are getting rid of the message forever. But I think that most people here are looking for..

          Show
          Dan Poltawski added a comment - I've implemented both of Sam (M)'s suggestions. Problem with this approach is that its hard to get the message back, its not clear that you are getting rid of the message forever. But I think that most people here are looking for..
          Hide
          Davo Smith added a comment -

          Would there be any mileage in adding something on the user profile page, along the lines of 'show all dismissed messages', that would re-enable this (and might well be useful in the future, if we added extra, dismissable, 'Welcome to Moodle, here is how to get started' messages, a bit like you get when you first log into the Wordpress admin panel)?

          Show
          Davo Smith added a comment - Would there be any mileage in adding something on the user profile page, along the lines of 'show all dismissed messages', that would re-enable this (and might well be useful in the future, if we added extra, dismissable, 'Welcome to Moodle, here is how to get started' messages, a bit like you get when you first log into the Wordpress admin panel)?
          Hide
          Martin Dougiamas added a comment -

          Nope, sorry. It's better but still too clunky IMO.

          We have all the power of YUI3 at our disposal. Why load all this stuff and not use it? Let's add a bit of eye candy to make people say "wow" since we can here.

          When "turn editing on" is clicked - add code to the page to fade in the message "above" the page, directly under the nav bar, and then fade away after a few seconds.

          This will NOT need to show on every page load if the code is only added to the page when the editing flag is turned on.

          Show
          Martin Dougiamas added a comment - Nope, sorry. It's better but still too clunky IMO. We have all the power of YUI3 at our disposal. Why load all this stuff and not use it? Let's add a bit of eye candy to make people say "wow" since we can here. When "turn editing on" is clicked - add code to the page to fade in the message "above" the page, directly under the nav bar, and then fade away after a few seconds. This will NOT need to show on every page load if the code is only added to the page when the editing flag is turned on.
          Hide
          Martin Dougiamas added a comment -

          (I do like the idea of a generic popup-help mechanism but we need to design that properly at another time)

          Show
          Martin Dougiamas added a comment - (I do like the idea of a generic popup-help mechanism but we need to design that properly at another time)
          Show
          Martin Dougiamas added a comment - hint http://yuilibrary.com/yui/docs/anim/basic.html
          Hide
          Davo Smith added a comment -

          I've got a patch here:
          repo: git://github.com/davosmith/moodle.git
          branch: MDL-33056_dnd_status_bar
          diff: https://github.com/davosmith/moodle/compare/MDL-33056_dnd_status_bar

          This adds a shadow and curved corners to the message, uses position: absolute, so it doesn't move everything else on the page down and fades in over 0.8 seconds, waits 5 seconds, then fades out over 2 seconds. This only appears immediately after you have clicked 'turn editing on' (via an extra URL param that is only set at that point).

          Show
          Davo Smith added a comment - I've got a patch here: repo: git://github.com/davosmith/moodle.git branch: MDL-33056 _dnd_status_bar diff: https://github.com/davosmith/moodle/compare/MDL-33056_dnd_status_bar This adds a shadow and curved corners to the message, uses position: absolute, so it doesn't move everything else on the page down and fades in over 0.8 seconds, waits 5 seconds, then fades out over 2 seconds. This only appears immediately after you have clicked 'turn editing on' (via an extra URL param that is only set at that point).
          Hide
          Davo Smith added a comment -

          Completed 'stealing' from Dan P (as requested)

          Show
          Davo Smith added a comment - Completed 'stealing' from Dan P (as requested)
          Hide
          Martin Dougiamas added a comment -

          Yeah, just tried it out! Juicy!

          Tweaks I would do:

          1) Reduce the width a bit (ideally just around the text but I figure that might be difficult).
          2) Fade out and fade in can be snappy (0.5 seconds) too. Currently the page "feels" sluggish.
          3) Reduce 5 seconds to 2 seconds. I appreciate you'd want the work appreciated but it's very quick to read.

          Show
          Martin Dougiamas added a comment - Yeah, just tried it out! Juicy! Tweaks I would do: 1) Reduce the width a bit (ideally just around the text but I figure that might be difficult). 2) Fade out and fade in can be snappy (0.5 seconds) too. Currently the page "feels" sluggish. 3) Reduce 5 seconds to 2 seconds. I appreciate you'd want the work appreciated but it's very quick to read.
          Hide
          Dan Poltawski added a comment -

          My two cents: I think if it works like that popup/fade it'd be better right at the top of the page above the course header, there is less actual things its hoevering to get in the way of. It reminds me notification that pop down on iOS and interupt controls, really annoys me!

          Show
          Dan Poltawski added a comment - My two cents: I think if it works like that popup/fade it'd be better right at the top of the page above the course header, there is less actual things its hoevering to get in the way of. It reminds me notification that pop down on iOS and interupt controls, really annoys me!
          Hide
          Martin Dougiamas added a comment -

          I tried it with Fusion theme and it very nearly is up the top there anyway (Shadow is slightly dodgy on dark backgrounds too)

          OK, so how about doing that then? Right at the top of the page. Bonus points for having it slide down and backup like a little notification.

          Show
          Martin Dougiamas added a comment - I tried it with Fusion theme and it very nearly is up the top there anyway (Shadow is slightly dodgy on dark backgrounds too) OK, so how about doing that then? Right at the top of the page. Bonus points for having it slide down and backup like a little notification.
          Hide
          Davo Smith added a comment -

          I've pushed through Martin's suggestions (although I've set it to display for 3 seconds, as 2 seconds felt just a bit too quick - especially for the first time you see it).

          Dan - I don't think it really gets in the way much on the page and I'd be more worried that the message would be very out of place at the top of the page, if there is a fancy theme in use that had a picture or something complex up there. Anyway, it only appears when you switch on editing, so should only be seen once per session, by most users.

          Show
          Davo Smith added a comment - I've pushed through Martin's suggestions (although I've set it to display for 3 seconds, as 2 seconds felt just a bit too quick - especially for the first time you see it). Dan - I don't think it really gets in the way much on the page and I'd be more worried that the message would be very out of place at the top of the page, if there is a fancy theme in use that had a picture or something complex up there. Anyway, it only appears when you switch on editing, so should only be seen once per session, by most users.
          Hide
          Martin Dougiamas added a comment -

          After experimenting I'd go for 0.2 secs -> 2secs -> 0.2 secs

          Show
          Martin Dougiamas added a comment - After experimenting I'd go for 0.2 secs -> 2secs -> 0.2 secs
          Hide
          Davo Smith added a comment -

          I've just moved it to the top of the page and added a 'slide up and down' movement (and adjusted the rounded corners) - do you want to check that version, before I tweak the timings again?

          Show
          Davo Smith added a comment - I've just moved it to the top of the page and added a 'slide up and down' movement (and adjusted the rounded corners) - do you want to check that version, before I tweak the timings again?
          Hide
          Dan Poltawski added a comment -

          Love it.

          Show
          Dan Poltawski added a comment - Love it.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          This is a massive improvement thank you and has been integrated now!
          I have been playing with theme's a lot in the past couple of days and that status box had been continually catching my eye.

          The only thing I noted was that add_status_div is using a lot of classic JS rather than YUI for basic handling.
          It'd be nice to fully utilise YUI there (seeing as we have the Y instance) and it would negate the current jump that users may see in IE8 because div.style.opacity is not supported until IE9.
          Not really a concern as on a fast machine you won't even notice it, the opacity gets properly set by the animation as soon as it is initialised a few lines later.
          But for later.. YUI over classic JS please

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, This is a massive improvement thank you and has been integrated now! I have been playing with theme's a lot in the past couple of days and that status box had been continually catching my eye. The only thing I noted was that add_status_div is using a lot of classic JS rather than YUI for basic handling. It'd be nice to fully utilise YUI there (seeing as we have the Y instance) and it would negate the current jump that users may see in IE8 because div.style.opacity is not supported until IE9. Not really a concern as on a fast machine you won't even notice it, the opacity gets properly set by the animation as soon as it is initialised a few lines later. But for later.. YUI over classic JS please Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Heh just realised no testing instructions.
          Not a problem however I tested this during integration.
          Was tested on several browsers in several themes. Only core theme not working was mymobile (doesn't have #page) but that doesn't support editing and I don't imagine it supports drag and drop so no probs.

          Show
          Sam Hemelryk added a comment - Heh just realised no testing instructions. Not a problem however I tested this during integration. Was tested on several browsers in several themes. Only core theme not working was mymobile (doesn't have #page) but that doesn't support editing and I don't imagine it supports drag and drop so no probs.
          Hide
          Dan Poltawski added a comment -

          Just noticed this wasn't working on Michael D's machine (firefox, windows). Investigating.

          Show
          Dan Poltawski added a comment - Just noticed this wasn't working on Michael D's machine (firefox, windows). Investigating.
          Hide
          Dan Poltawski added a comment -

          (Works for me on firefox on windows)

          Show
          Dan Poltawski added a comment - (Works for me on firefox on windows)
          Hide
          Dan Poltawski added a comment -

          Oh. I notice in this issue that an additional get param 'editingenabled' has crept in.

          Why was this necessary? We have $PAGE->user_is_editing()

          Show
          Dan Poltawski added a comment - Oh. I notice in this issue that an additional get param 'editingenabled' has crept in. Why was this necessary? We have $PAGE->user_is_editing()
          Hide
          Dan Poltawski added a comment -

          I guess it was to stop the message continually appearing, but I REALLY don't like this solution.

          Its adding additional clutter to the page url and I can imagine that some third party authors might start depending on it.

          People might be tricked into thinking that calling:
          http://moodle/course/view.php?id=2&editingenabled=1

          will enable editing.

          Show
          Dan Poltawski added a comment - I guess it was to stop the message continually appearing, but I REALLY don't like this solution. Its adding additional clutter to the page url and I can imagine that some third party authors might start depending on it. People might be tricked into thinking that calling: http://moodle/course/view.php?id=2&editingenabled=1 will enable editing.
          Hide
          Davo Smith added a comment -

          Dan - What is the alternative?

          Sam - yes, I will look at replacing the code with YUI calls, at some point. IE8 compatibility is irrelevant in this case, as IE8 (and IE9) does not support drag and drop, so the message will never appear.

          Show
          Davo Smith added a comment - Dan - What is the alternative? Sam - yes, I will look at replacing the code with YUI calls, at some point. IE8 compatibility is irrelevant in this case, as IE8 (and IE9) does not support drag and drop, so the message will never appear.
          Hide
          Martin Dougiamas added a comment -

          Is it a POST/GET thing? Can it just be a variable in POST?

          Show
          Martin Dougiamas added a comment - Is it a POST/GET thing? Can it just be a variable in POST?
          Hide
          Davo Smith added a comment -

          The button does a post to turn on editing, then a redirect to the course page (without the post variables).

          I added an extra variable onto the end of the redirect to indicate that the notification should be shown. I don't think redirects can include POST params (but I'm ready to be corrected on that front).

          The other solution I can think of was to set something in the $SESSION global, then clear it as soon as it is read, but I thought that was even messier than adding an extra GET param to the redirect.

          Show
          Davo Smith added a comment - The button does a post to turn on editing, then a redirect to the course page (without the post variables). I added an extra variable onto the end of the redirect to indicate that the notification should be shown. I don't think redirects can include POST params (but I'm ready to be corrected on that front). The other solution I can think of was to set something in the $SESSION global, then clear it as soon as it is read, but I thought that was even messier than adding an extra GET param to the redirect.
          Hide
          Dan Poltawski added a comment -

          Neither is ideal, but the reason I don't like the get param is just because its very visible.

          Be interested to hear what sam thinks.

          Show
          Dan Poltawski added a comment - Neither is ideal, but the reason I don't like the get param is just because its very visible. Be interested to hear what sam thinks.
          Hide
          Sam Hemelryk added a comment -

          Howdy,

          Yip the new param caught my eye as well, in the end it got in because I referred back to what was asked for and could not think of any better solution.

          • Post is out because you can't redirect and ask the page to include a post variable. Would have to be part of the URL hence GET.
          • Session is a possibility, but I personally feel it is much worse than using GET. We can be sure that the GET param only exists for the request. However if anything goes wrong, or even not as we expected with the session param then we leave clutter in the session. Small as it may be.,
          • Killing the redirect would be ideal so that it was not needed. But thats no small change and would require serious thinking.

          The visibility of the GET is as Dan mentions what is unpleasant, also worth mentioning is that certainly we don't want to be doing this sort of thing any more than we absolutely need to. It clutters the URL and despite being a single request param could really lead to confusing code.
          At the moment this is the best solution in my eyes given what was asked for, and as such it got in.
          There are three paths we can go down now:

          1. Live with it as it is presently. Not great it would be nice to find a better solution.
          2. Agree on a better solution to achieve the same thing, implement it and integrate it when we've got it done.
          3. Change what we are trying to do. Show the message on every course page load, or something else. Who knows. Create a patch to revert this and implement the alternative idea.

          Personally I think perhaps now is the time to revamp our turn on editing functionality so that a: we don't need the redirect (only state is changing) and b: all turn on editing to occur on any page (essentially taking you back to that page).

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Howdy, Yip the new param caught my eye as well, in the end it got in because I referred back to what was asked for and could not think of any better solution. Post is out because you can't redirect and ask the page to include a post variable. Would have to be part of the URL hence GET. Session is a possibility, but I personally feel it is much worse than using GET. We can be sure that the GET param only exists for the request. However if anything goes wrong, or even not as we expected with the session param then we leave clutter in the session. Small as it may be., Killing the redirect would be ideal so that it was not needed. But thats no small change and would require serious thinking. The visibility of the GET is as Dan mentions what is unpleasant, also worth mentioning is that certainly we don't want to be doing this sort of thing any more than we absolutely need to. It clutters the URL and despite being a single request param could really lead to confusing code. At the moment this is the best solution in my eyes given what was asked for, and as such it got in. There are three paths we can go down now: Live with it as it is presently. Not great it would be nice to find a better solution. Agree on a better solution to achieve the same thing, implement it and integrate it when we've got it done. Change what we are trying to do. Show the message on every course page load, or something else. Who knows. Create a patch to revert this and implement the alternative idea. Personally I think perhaps now is the time to revamp our turn on editing functionality so that a: we don't need the redirect (only state is changing) and b: all turn on editing to occur on any page (essentially taking you back to that page). Cheers Sam
          Hide
          Dan Poltawski added a comment -

          Another possibility would be to change the name of the param. One of the main reasons I don't like it is because its looks to a casual observer that it is the param which changes the editing behaviour of the page (?edit=1). As usual with things like this, I don't have good suggestions for other names myself!

          Another thing to throw in the mix.. MD indicates a preference to get rid of the editing button entirely in the future. How will we do it then..

          Show
          Dan Poltawski added a comment - Another possibility would be to change the name of the param. One of the main reasons I don't like it is because its looks to a casual observer that it is the param which changes the editing behaviour of the page (?edit=1). As usual with things like this, I don't have good suggestions for other names myself! Another thing to throw in the mix.. MD indicates a preference to get rid of the editing button entirely in the future. How will we do it then..
          Hide
          Sam Hemelryk added a comment -

          Changing the name is a goody, what about notifyeditingon, or if you want to make sure its not mistaken something like notifystate or notifystatechange.

          We have the link in the settings navigation to turn editing on, the idea was originally to remove all turn on editing buttons and move everything to the settings navigation. It has not happened yet because it was met with objections. I think at one point in fact many had been removed but were readded in some circumstances.
          Certainly it would be great to have a single "Turn on editing" link in the settings navigation for all pages providing you have capability to edit for the current context (settings navigation being context based that shouldn't be a problem). Then we can do away with all other buttons + links knowing we have one consistent link in the settings nav at all time.
          I think at that point we would need to have the redirect removed.

          Show
          Sam Hemelryk added a comment - Changing the name is a goody, what about notifyeditingon, or if you want to make sure its not mistaken something like notifystate or notifystatechange. We have the link in the settings navigation to turn editing on, the idea was originally to remove all turn on editing buttons and move everything to the settings navigation. It has not happened yet because it was met with objections. I think at one point in fact many had been removed but were readded in some circumstances. Certainly it would be great to have a single "Turn on editing" link in the settings navigation for all pages providing you have capability to edit for the current context (settings navigation being context based that shouldn't be a problem). Then we can do away with all other buttons + links knowing we have one consistent link in the settings nav at all time. I think at that point we would need to have the redirect removed.
          Hide
          Dan Poltawski added a comment -

          notifyeditingon makes sense and so I have gone ahead and implemented that in MDL-33593

          Show
          Dan Poltawski added a comment - notifyeditingon makes sense and so I have gone ahead and implemented that in MDL-33593
          Hide
          Sam Hemelryk added a comment -

          Great thanks Dan

          Show
          Sam Hemelryk added a comment - Great thanks Dan
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: