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

SCORM user report trak details table not wrapping the data

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.2, 2.1, 2.1.1, 2.1.3, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3
    • Component/s: SCORM
    • Environment:
      Windows
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      Upload an SCORM package ( In my case my course have a quiz which has 'short answer' question.). Complete the course and provide answers/input to short answer/questions. Submit the tracking data / suspend data.
      View the tracks details report for a submission and check to make sure that any fields with long data (look at suspend_data) wrap the text.
      Only modern browsers that support word-wrap/word-break will work.

      Show
      Upload an SCORM package ( In my case my course have a quiz which has 'short answer' question.). Complete the course and provide answers/input to short answer/questions. Submit the tracking data / suspend data. View the tracks details report for a submission and check to make sure that any fields with long data (look at suspend_data) wrap the text. Only modern browsers that support word-wrap/word-break will work.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE

      Description

      For SCORM user report when we click on 'track details' link. User tracking data is cut/hidden on right hand side as table does not wrap the data.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            danmarsden Dan Marsden added a comment -

            thanks for the report - I've put this in my stable backlog - if you manage to track down some css that fixes it feel free to attach a patch! - thanks.

            Show
            danmarsden Dan Marsden added a comment - thanks for the report - I've put this in my stable backlog - if you manage to track down some css that fixes it feel free to attach a patch! - thanks.
            Hide
            salvetore Michael de Raadt added a comment -

            I had a report that this is still affecting users.

            It would be good if you could look at it again, Dan.

            Show
            salvetore Michael de Raadt added a comment - I had a report that this is still affecting users. It would be good if you could look at it again, Dan.
            Hide
            domnitec Dom Seneque added a comment -

            Hi Dan,
            One of our client's is also experiencing this problem in the "Other Tracks" grid, where it disappears off the screen.

            Show
            domnitec Dom Seneque added a comment - Hi Dan, One of our client's is also experiencing this problem in the "Other Tracks" grid, where it disappears off the screen.
            Hide
            danmarsden Dan Marsden added a comment -

            Unassigning myself as I don't have plans to look at this right now as a volunteer - I'm happy to review any patches.

            Show
            danmarsden Dan Marsden added a comment - Unassigning myself as I don't have plans to look at this right now as a volunteer - I'm happy to review any patches.
            Hide
            danmarsden Dan Marsden added a comment -

            took a look at this today as I was working on user reports and thought it made sense to dump that large descriptor column and use help icons beside each value instead - this has the advantage of making the table a lot less bulky and will hopefully help a lot.

            Show
            danmarsden Dan Marsden added a comment - took a look at this today as I was working on user reports and thought it made sense to dump that large descriptor column and use help icons beside each value instead - this has the advantage of making the table a lot less bulky and will hopefully help a lot.
            Hide
            danmarsden Dan Marsden added a comment -

            note to reviewer - please check AMOS script - you will see that not all strings have been moved as some have new text quite different from the old text so I have deliberately left them out of the AMOS script.

            Show
            danmarsden Dan Marsden added a comment - note to reviewer - please check AMOS script - you will see that not all strings have been moved as some have new text quite different from the old text so I have deliberately left them out of the AMOS script.
            Hide
            ankit_frenz Ankit Agarwal added a comment - - edited

            Hi Dan,
            Thanks for improving the user report. The patch looks great, here are a few minor suggestion:-

            1. (optional) We should have spaces around '='. Since the patch is rewriting those lines I would recommend making this improvement at the same time.
            2. Am not sure about this, but are we not degrading usability by introducing extra clicks? As I assume the interactions name will make no sense to teachers what so ever, so now they have to click on each interaction to understand what it is.
            3. The actual issue of wrapping is still present. It is because of :-

              $table->wrap = array('nowrap', 'wrap');
              

              That is an incorrect usage of "wrap" property. That code actually adds a no-wrap property to the td for the second column (have a look at html_writer::table()). However removing it, also doesn't really solve the issue completely. Once it is removed, we have wrapping at line breaks, however still data can be lost. (Attached screens). May be we should look into a css solution?

            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - - edited Hi Dan, Thanks for improving the user report. The patch looks great, here are a few minor suggestion:- (optional) We should have spaces around '='. Since the patch is rewriting those lines I would recommend making this improvement at the same time. Am not sure about this, but are we not degrading usability by introducing extra clicks? As I assume the interactions name will make no sense to teachers what so ever, so now they have to click on each interaction to understand what it is. The actual issue of wrapping is still present. It is because of :- $table->wrap = array('nowrap', 'wrap'); That is an incorrect usage of "wrap" property. That code actually adds a no-wrap property to the td for the second column (have a look at html_writer::table()). However removing it, also doesn't really solve the issue completely. Once it is removed, we have wrapping at line breaks, however still data can be lost. (Attached screens). May be we should look into a css solution? Thanks
            Hide
            danmarsden Dan Marsden added a comment -

            Hi Ankit - the biggest issue with wrapping is actually the suspend_data field - it stores a text string (up to 4096 chars for SCORM 1.2) but usually doesn't contain any spaces.

            getting text that doesn't have spaces to wrap isn't an easy thing to fix using css - I've had a quick look to see if there's a reliable way to wrap text that has no spaces and I couldn't see a method using css that is supported in all browsers. I think the best option might be to truncate the suspend_data field when displaying on the page and giving some form of visual indicator to show that this has happened. I'm not sure if I'll get time to fix this right now so if I can't do that I'll shift this particular patch onto a separate bug and submit it there.

            To understand SCORM tracking data requires some level of knowledge - we're increasing usability by making the data more viewable - the same SCORM data type also appears multiple times so repeating the descriptor multiple times in the table doesn't make sense to me either. The table in it's current form isn't very usable as you need to scroll horizontally past the descriptions to see the data - moving the descriptors out of the table and into help pop-ups makes it much cleaner.

            I also have a patch incoming that makes further improvements to the user reports by adding a bunch of other stuff.

            Show
            danmarsden Dan Marsden added a comment - Hi Ankit - the biggest issue with wrapping is actually the suspend_data field - it stores a text string (up to 4096 chars for SCORM 1.2) but usually doesn't contain any spaces. getting text that doesn't have spaces to wrap isn't an easy thing to fix using css - I've had a quick look to see if there's a reliable way to wrap text that has no spaces and I couldn't see a method using css that is supported in all browsers. I think the best option might be to truncate the suspend_data field when displaying on the page and giving some form of visual indicator to show that this has happened. I'm not sure if I'll get time to fix this right now so if I can't do that I'll shift this particular patch onto a separate bug and submit it there. To understand SCORM tracking data requires some level of knowledge - we're increasing usability by making the data more viewable - the same SCORM data type also appears multiple times so repeating the descriptor multiple times in the table doesn't make sense to me either. The table in it's current form isn't very usable as you need to scroll horizontally past the descriptions to see the data - moving the descriptors out of the table and into help pop-ups makes it much cleaner. I also have a patch incoming that makes further improvements to the user reports by adding a bunch of other stuff.
            Hide
            danmarsden Dan Marsden added a comment -

            shifting this work to MDL-41290 - I've also managed to sort the wrapping issue with suspend data there in most browsers I've checked using:

            word-wrap:break-word,word-break: break-all;

            Show
            danmarsden Dan Marsden added a comment - shifting this work to MDL-41290 - I've also managed to sort the wrapping issue with suspend data there in most browsers I've checked using: word-wrap:break-word,word-break: break-all;
            Hide
            danmarsden Dan Marsden added a comment -

            NOTE TO INTEGRATOR:
            This is a stable only patch (24/25) that is blocked by MDL-41290 which must be integrated in master before this is integrated.

            MDL-41290 is a master only change with a big rewrite of the user reports and contains this change among a range of other user report improvements.

            coding style of scorm/styles.css is also fixed in master.

            Show
            danmarsden Dan Marsden added a comment - NOTE TO INTEGRATOR: This is a stable only patch (24/25) that is blocked by MDL-41290 which must be integrated in master before this is integrated. MDL-41290 is a master only change with a big rewrite of the user reports and contains this change among a range of other user report improvements. coding style of scorm/styles.css is also fixed in master.
            Hide
            poltawski Dan Poltawski added a comment -

            This is currently blocked.

            Show
            poltawski Dan Poltawski added a comment - This is currently blocked.
            Hide
            damyon Damyon Wiese added a comment -

            Stealing from Dan (looks like was just left on from last week).

            Show
            damyon Damyon Wiese added a comment - Stealing from Dan (looks like was just left on from last week).
            Hide
            damyon Damyon Wiese added a comment -

            At first I thought this wasn't working correctly because it was wrapping in the middle of words until I read all the comments.

            This does do what is intended, and I guess because of that suspend_data field, this at least makes the text visible.

            IMO the suspend_data field should not be shown in full in the table, and should be a link to show the text in a popup/new window. Or - the table cells should be fixed size with overflow-scroll. Leaving these here as suggestions for further work.

            Show
            damyon Damyon Wiese added a comment - At first I thought this wasn't working correctly because it was wrapping in the middle of words until I read all the comments. This does do what is intended, and I guess because of that suspend_data field, this at least makes the text visible. IMO the suspend_data field should not be shown in full in the table, and should be a link to show the text in a popup/new window. Or - the table cells should be fixed size with overflow-scroll. Leaving these here as suggestions for further work.
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Dan,

            Integrated to 24 and 25 only.

            Show
            damyon Damyon Wiese added a comment - Thanks Dan, Integrated to 24 and 25 only.
            Hide
            danmarsden Dan Marsden added a comment -

            thanks Damyon - most themes did allow some form of horizontal scrolling but people didn't like it. I agree that suspend_data should probably not be shown at all and use some form of pop-up to show the text - but that pop-up wouldn't be very nice either as the text could still be way too wide. I was tempted to just truncate it and only show the first X characters in the html view but the css hack was easier than I thought it was going to be.

            Show
            danmarsden Dan Marsden added a comment - thanks Damyon - most themes did allow some form of horizontal scrolling but people didn't like it. I agree that suspend_data should probably not be shown at all and use some form of pop-up to show the text - but that pop-up wouldn't be very nice either as the text could still be way too wide. I was tempted to just truncate it and only show the first X characters in the html view but the css hack was easier than I thought it was going to be.
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success!

            Tested in 2.4, 2.5 and master using Firefox.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success! Tested in 2.4, 2.5 and master using Firefox.
            Hide
            damyon Damyon Wiese added a comment -

            This issue along with 77 of it's friends has been sent upstream and released to the world.

            Thankyou for your contribution.

            Show
            damyon Damyon Wiese added a comment - This issue along with 77 of it's friends has been sent upstream and released to the world. Thankyou for your contribution.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Nov/13