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

Show which user profile field is being deleted

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1
    • Fix Version/s: 2.2
    • Component/s: Usability
    • Labels:

      Description

      There are 2 usability problems related to the management of the custom user profile fields:
      1. On the wide screen, the names of the fields are far away from the editing icons (/user/profile/index.php). It makes it a bit hard to "aim" for the field you want to edit (see user_profile_field_usability-1.jpg screenshot).
      2. On the delete confirmation page (user_profile_field_usability-2.jpg) there is no mention which field is actually being deleted.

      I suggest fixing point 2 by adding the name of the field being deleted into the message.

        Gliffy Diagrams

          Activity

          Hide
          skodak Petr Skoda added a comment -

          Hello, this change is not optimal for stable branches because we should not add new $a placeholders there.

          Show
          skodak Petr Skoda added a comment - Hello, this change is not optimal for stable branches because we should not add new $a placeholders there.
          Hide
          tmuras Tomasz Muras added a comment -

          Hi Petr,

          It's more of an improvement than a bug, so let's add that to 2.2 only.

          Show
          tmuras Tomasz Muras added a comment - Hi Petr, It's more of an improvement than a bug, so let's add that to 2.2 only.
          Hide
          skodak Petr Skoda added a comment -

          Thanks!

          Show
          skodak Petr Skoda added a comment - Thanks!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited

          Do we really want new " uses there? Why not simply use " and done? I know there are some more "wrong" uses, but the 99.9 is using quotes afaik.

          Also, changing the "structure" of one existing string (by adding params to it...) wasn't the sort of thing to be avoided? Or that's not true anymore since the born of AMOS?

          If that is not problematic anymore... why can this be backported to STABLEs? It is an improvement but for something really missing to be usable.

          Adding David to clarify and learn about such type of changes...TIA!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited Do we really want new " uses there? Why not simply use " and done? I know there are some more "wrong" uses, but the 99.9 is using quotes afaik. Also, changing the "structure" of one existing string (by adding params to it...) wasn't the sort of thing to be avoided? Or that's not true anymore since the born of AMOS? If that is not problematic anymore... why can this be backported to STABLEs? It is an improvement but for something really missing to be usable. Adding David to clarify and learn about such type of changes...TIA!
          Hide
          skodak Petr Skoda added a comment -

          Hmm, right, the "" is the correct way. The changing is not a problem in master branch, translators are notified when something is changed. The worst case scenario is that the field name will not be in the translated pack.

          Show
          skodak Petr Skoda added a comment - Hmm, right, the "" is the correct way. The changing is not a problem in master branch, translators are notified when something is changed. The worst case scenario is that the field name will not be in the translated pack.
          Hide
          skodak Petr Skoda added a comment -

          I am going to update the pull today, thanks.

          Show
          skodak Petr Skoda added a comment - I am going to update the pull today, thanks.
          Hide
          skodak Petr Skoda added a comment -

          Oh, there are many "s already, I suppose we could get rid of all of them if necessary.

          Show
          skodak Petr Skoda added a comment - Oh, there are many "s already, I suppose we could get rid of all of them if necessary.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          +1 to modify the rest of & quot; uses in separate issue. For you consideration if you want to modify the new one here or in the separate issue.

          Ok, so in master we can change any string completely, adding params, changing meaning... anything.

          Ciao

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - +1 to modify the rest of & quot; uses in separate issue. For you consideration if you want to modify the new one here or in the separate issue. Ok, so in master we can change any string completely, adding params, changing meaning... anything. Ciao
          Hide
          mudrd8mz David Mudrak added a comment -

          1. Definitely no &quotes please. Use proper single quotes in English texts (as native speakers seem to prefer them over double quotes in Moodle - but that can be decided and then used consistently for ever)
          2. Eloy, the problem with adding/removing placeholders in string on STABLE branches is that if folks update Moodle but they do not update their lang packs then Moodle relies on different string structure than is present there, even if the maintainer fixes the string as soon as AMOS sends the notification. That would be fixed by MDL-29038 though. But still there is another issue: admins may update their lang packs for the stable branch without updating Moodle to the most recent version. Then the lang pack is valid for the most recent version while their Moodle may expect different version. Therefore the recommendation was not to remove strings or modify placeholders on stable branches. Although using the most recent version of Moodle code and having the maintainer who looks after the pack carefully.

          Show
          mudrd8mz David Mudrak added a comment - 1. Definitely no &quotes please. Use proper single quotes in English texts (as native speakers seem to prefer them over double quotes in Moodle - but that can be decided and then used consistently for ever) 2. Eloy, the problem with adding/removing placeholders in string on STABLE branches is that if folks update Moodle but they do not update their lang packs then Moodle relies on different string structure than is present there, even if the maintainer fixes the string as soon as AMOS sends the notification. That would be fixed by MDL-29038 though. But still there is another issue: admins may update their lang packs for the stable branch without updating Moodle to the most recent version. Then the lang pack is valid for the most recent version while their Moodle may expect different version. Therefore the recommendation was not to remove strings or modify placeholders on stable branches. Although using the most recent version of Moodle code and having the maintainer who looks after the pack carefully.
          Hide
          tsala Helen Foster added a comment -

          Re 1. I vote for single quotes, as used in British and Australian English.

          Show
          tsala Helen Foster added a comment - Re 1. I vote for single quotes, as used in British and Australian English.
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Thanks for feedback!

          I've added one extra commit replacing 2 occurrences of the entity by the single quote as exposed. The 2 strings were related to profile fields are are:

          • profilecreatenewfield
          • profilecreatenewfield

          Note there are still a lot of quot entities everywhere. Feel free to create issue for fixing that.

          Integrated, thanks!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for feedback! I've added one extra commit replacing 2 occurrences of the entity by the single quote as exposed. The 2 strings were related to profile fields are are: profilecreatenewfield profilecreatenewfield Note there are still a lot of quot entities everywhere. Feel free to create issue for fixing that. Integrated, thanks!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          While changing the strings as commented above, I've tested this. The name of the field is shown properly on deletion.

          Passing.

          BTW: Testing instructions were missing, don't forget they are mandatory, lol. I just forgot it this time.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - While changing the strings as commented above, I've tested this. The name of the field is shown properly on deletion. Passing. BTW: Testing instructions were missing, don't forget they are mandatory, lol. I just forgot it this time.
          Hide
          andyjdavis Andrew Davis added a comment -

          testing instructions?

          Show
          andyjdavis Andrew Davis added a comment - testing instructions?
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited

          hehe, I won by 1 minute, Andrew!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - - edited hehe, I won by 1 minute, Andrew!
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work, this has been sent upstream and is available in all the git and cvs repositories.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                5/Dec/11