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
    • Rank:
      41304

      Description

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

        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: