Moodle
  1. Moodle
  2. MDL-32999 META: Files UI Stage 2 polishing in master
  3. MDL-33425

Replace filepicker form element with filemanager in Edit user profile form

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.3
    • Component/s: Forms Library
    • Labels:
      None
    • Testing Instructions:
      Hide

      Test 1

      1. Log in as admin
      2. Go to edit profile page
      3. Change user picture and make sure it gets updated

      Test 2

      1. Log in as admin
      2. Edit other user profile
      3. Change user picture and make sure it gets updated

      Test 3

      1. Log in as admin
      2. Create a new user and make sure to add user picture while creating new account
      3. Make sure user picture is added to new user profile

      Test 4

      1. Log in as student
      2. edit profile and change user picture
      3. make sure it gets updated.
      Show
      Test 1 Log in as admin Go to edit profile page Change user picture and make sure it gets updated Test 2 Log in as admin Edit other user profile Change user picture and make sure it gets updated Test 3 Log in as admin Create a new user and make sure to add user picture while creating new account Make sure user picture is added to new user profile Test 4 Log in as student edit profile and change user picture make sure it gets updated.
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-mdl-33425

      Description

      This is the most visible occurrence of strange filepicker form element. People keep asking why it looks different from nice new filemanager

        Gliffy Diagrams

          Activity

          Hide
          Marina Glancy added a comment -

          Don't forget to specify accepted_types=web_image

          Show
          Marina Glancy added a comment - Don't forget to specify accepted_types=web_image
          Hide
          Rajesh Taneja added a comment -

          I had a word with Marina about removing "Current picture" and "Delete" checkbox. At this point I am not sure if this should be removed as it might be confusing for few people. If required will handle that in different bug.

          FYI:
          As file manager doesn't provide path for uploaded file, and process_new_icon needs image path. I choose to copy file in temp location and then use it, rather then modifying process_new_icon or introducing another variation of process_new_icon.

          Show
          Rajesh Taneja added a comment - I had a word with Marina about removing "Current picture" and "Delete" checkbox. At this point I am not sure if this should be removed as it might be confusing for few people. If required will handle that in different bug. FYI: As file manager doesn't provide path for uploaded file, and process_new_icon needs image path. I choose to copy file in temp location and then use it, rather then modifying process_new_icon or introducing another variation of process_new_icon.
          Hide
          Frédéric Massart added a comment -

          Hey Raj, nice work there.

          Feel free to submit for integration whenever you're ready. However, you might want to update user/editadvanced.php:148 and user/edit.php:166 with a double identation.

          In lib/filestorage/stored_file.php, I would wrap the if statement in one more parentheses pair, but that's just me.

          Thanks!

          Show
          Frédéric Massart added a comment - Hey Raj, nice work there. Feel free to submit for integration whenever you're ready. However, you might want to update user/editadvanced.php:148 and user/edit.php:166 with a double identation. In lib/filestorage/stored_file.php, I would wrap the if statement in one more parentheses pair, but that's just me. Thanks!
          Hide
          Rajesh Taneja added a comment -

          Thanks Fred,

          As discussed, these changes are not required, hence pushing it for integration.

          Show
          Rajesh Taneja added a comment - Thanks Fred, As discussed, these changes are not required, hence pushing it for integration.
          Hide
          Sam Hemelryk added a comment -

          Alrighty, this has been integrated now thanks guys.
          I did a quick bit of testing during integration and noted that I couldn't drag/drop an image onto that file picker. It appears that ajax upload doesn't handle the web_image, or more explicitly any "grouped" mimetypes. Looked to me like a separate bug so I ignored it but does anyone know whether these is an issue for that?

          Show
          Sam Hemelryk added a comment - Alrighty, this has been integrated now thanks guys. I did a quick bit of testing during integration and noted that I couldn't drag/drop an image onto that file picker. It appears that ajax upload doesn't handle the web_image, or more explicitly any "grouped" mimetypes. Looked to me like a separate bug so I ignored it but does anyone know whether these is an issue for that?
          Hide
          Rajesh Taneja added a comment -

          Thanks Sam,

          Yes, I tried drag-n-drop and it was not working. Sorry, forgot to mention that in comments. Will open another bug for that.

          Show
          Rajesh Taneja added a comment - Thanks Sam, Yes, I tried drag-n-drop and it was not working. Sorry, forgot to mention that in comments. Will open another bug for that.
          Hide
          Ankit Agarwal added a comment -

          Working as expected except the drag and drop thing mentioned.
          passing.
          Thanks

          Show
          Ankit Agarwal added a comment - Working as expected except the drag and drop thing mentioned. passing. Thanks
          Hide
          Martin Dougiamas added a comment -

          Ha, came here to mention I also noticed drag/drop was not accepting any files.

          Show
          Martin Dougiamas added a comment - Ha, came here to mention I also noticed drag/drop was not accepting any files.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay!

          Many, many thanks for your hard work!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And this has been incorporated to all the weekly builds and also, to Moodle 2.3 Release Candidate 1, yay! Many, many thanks for your hard work! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: