Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.5
    • Component/s: Administration
    • Labels:
    • Testing Instructions:
      Hide

      Make sure the following admin table has id and containns the following class: 'admintable generaltable'
      Also make sure there's is no inline style for the table

      1. go to Site admin > server > environment
      2. go to Site admin > users > permissions > assign system roles
        • review the table id and class
      3. go to Site admin > users > permissions > assign system roles
        • select a role
        • review the table id and class
      4. go to Site admin > users > permissions > Define roles
      5. go to Site admin > Development > XMLDB editor
      6. go to Site admin > users > accounts > browse list of users
      7. go to Site admin > users > accounts > cohorts
      8. go to Site admin > reports > comments
      9. go to Site admin > courses > add/edit courses
      10. go to Site admin > reports > config changes
      11. go to Site admin > reports > security overview
      Show
      Make sure the following admin table has id and containns the following class: 'admintable generaltable' Also make sure there's is no inline style for the table go to Site admin > server > environment go to Site admin > users > permissions > assign system roles review the table id and class go to Site admin > users > permissions > assign system roles select a role review the table id and class go to Site admin > users > permissions > Define roles go to Site admin > Development > XMLDB editor go to Site admin > users > accounts > browse list of users go to Site admin > users > accounts > cohorts go to Site admin > reports > comments go to Site admin > courses > add/edit courses go to Site admin > reports > config changes go to Site admin > reports > security overview
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      46029

      Description

      Add consistency for admin setting table to have ID and class name.

        Issue Links

          Activity

          Hide
          Jason Fowler added a comment -

          [ ] Syntax
          [ ] Output
          [ ] Whitespace
          [ ] Language
          [ ] Databases
          [ ] Testing
          [ ] Security
          [ ] Documentation
          [ ] Git
          [ ] Sanity check

          Some of the lines are pretty long - didn't count the characters, so I'm not sure if they are too long or not, just thought I'd mention it for your consideration.

          Other than that, it's all great Rossi.

          Show
          Jason Fowler added a comment - [ ] Syntax [ ] Output [ ] Whitespace [ ] Language [ ] Databases [ ] Testing [ ] Security [ ] Documentation [ ] Git [ ] Sanity check Some of the lines are pretty long - didn't count the characters, so I'm not sure if they are too long or not, just thought I'd mention it for your consideration. Other than that, it's all great Rossi.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Jason for reviewing.

          I will leave the patch as it is for now.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Jason for reviewing. I will leave the patch as it is for now. Submitting for integration review.
          Hide
          Dan Poltawski added a comment -

          Adding integration_held, as this looks like a master only change.

          Show
          Dan Poltawski added a comment - Adding integration_held, as this looks like a master only change.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (master-only), thanks!

          Side note: I'm not sure about the purpose of all those summary="" attributes. It could give some information to accessibility-readers but, in the other side, they are not supported by html5... so I'd suggest you to raise one discussion about them and, once decided, perform accordingly (deleting them, or filling them with real information... or whatever).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (master-only), thanks! Side note: I'm not sure about the purpose of all those summary="" attributes. It could give some information to accessibility-readers but, in the other side, they are not supported by html5... so I'd suggest you to raise one discussion about them and, once decided, perform accordingly (deleting them, or filling them with real information... or whatever). Ciao
          Hide
          David Monllaó added a comment -

          It passes, all tables contains id and the mentioned classes

          Show
          David Monllaó added a comment - It passes, all tables contains id and the mentioned classes
          Hide
          Dan Poltawski added a comment -

          Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

          Show
          Dan Poltawski added a comment - Hurray! We did it! Thanks to all the reporters, testers, user and watchers for a bumper week of Moodling!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: