Details

    • Testing Instructions:
      Hide

      I'm not sure if this needs testing as it is just renaming a css class which is predefined for the purpose it is meant for. So the tables in this change will be consistent with other tables in Moodle.

      Show
      I'm not sure if this needs testing as it is just renaming a css class which is predefined for the purpose it is meant for. So the tables in this change will be consistent with other tables in Moodle.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-37613_master

      Description

      Affected files...

      • mod/feedback/analysis.php
      • mod/feedback/analysis_course.php
      • mod/glossary/editcategories.html
      • mod/glossary/editcategories.php
      • mod/glossary/formats.php
      • mod/wiki/pagelib.php (5 instances)

        Gliffy Diagrams

          Attachments

          1. categorieslist_generalbox.png
            categorieslist_generalbox.png
            18 kB
          2. categorieslist_generaltable.png
            categorieslist_generaltable.png
            18 kB
          3. feedback_generalbox.png
            feedback_generalbox.png
            28 kB
          4. feedback_generaltable.png
            feedback_generaltable.png
            28 kB
          5. glossaryformat_generalbox.png
            glossaryformat_generalbox.png
            53 kB
          6. glossaryformat_generaltable.png
            glossaryformat_generaltable.png
            54 kB
          7. glossary-generalbox.png
            glossary-generalbox.png
            11 kB
          8. glossary-generaltable.png
            glossary-generaltable.png
            17 kB
          9. wikicontrib_generalbox.png
            wikicontrib_generalbox.png
            26 kB
          10. wikicontrib_generaltable.png
            wikicontrib_generaltable.png
            21 kB

            Activity

            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            Hide
            lazydaisy Mary Evans added a comment -

            RE-BASED

            Show
            lazydaisy Mary Evans added a comment - RE-BASED
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Nice simple tidy up there thanks Mary.

            Show
            samhemelryk Sam Hemelryk added a comment - Nice simple tidy up there thanks Mary.
            Hide
            marina Marina Glancy added a comment -

            Hi, I opened the page mod/glossary/editcategories and I am not really sure we want to use generaltable there. See my screenshots. I have not tried other pages yet

            Show
            marina Marina Glancy added a comment - Hi, I opened the page mod/glossary/editcategories and I am not really sure we want to use generaltable there. See my screenshots. I have not tried other pages yet
            Hide
            marina Marina Glancy added a comment -

            I fail this test because in all cases the class "generalbox" was used for table in order to hide the fact that it is table because we don't want the lines between rows and columns. Changing the css class made the screens look worse.

            See attachments (if file name ends with "generalbox" this is before change, if it ends with "generaltable" this is after change)

            Show
            marina Marina Glancy added a comment - I fail this test because in all cases the class "generalbox" was used for table in order to hide the fact that it is table because we don't want the lines between rows and columns. Changing the css class made the screens look worse. See attachments (if file name ends with "generalbox" this is before change, if it ends with "generaltable" this is after change)
            Hide
            lazydaisy Mary Evans added a comment -

            Adding Amy Groshek so she can comment on this. I must admit that in this case the generalbox styling looks better. However I'll wait for Amy to decide.

            Show
            lazydaisy Mary Evans added a comment - Adding Amy Groshek so she can comment on this. I must admit that in this case the generalbox styling looks better. However I'll wait for Amy to decide.
            Hide
            agroshek Amy Groshek added a comment -

            Mary Evans I see what you're saying. I found this in some other places as well, when I looked at all the screenviews where this would have to be fixed. In many of these cases, I think the change to generaltable is fine overall. Problems occur where tables have been used to lay out items (buttons, for example, that can just be floated) that do not need to be in tables. IMHO, where we bring more consistency and simplicity to our markup, and that results in the page view looking bad, we need to ask whether the page layout is actually being handled correctly.

            Obviously it means even more work... and if there is someone in particular maintaining that mod that person should be involved. But do we want to establish some CSS guidelines and then revisit this with those persons in on the conversation? What's your opinion?

            Show
            agroshek Amy Groshek added a comment - Mary Evans I see what you're saying. I found this in some other places as well, when I looked at all the screenviews where this would have to be fixed. In many of these cases, I think the change to generaltable is fine overall. Problems occur where tables have been used to lay out items (buttons, for example, that can just be floated) that do not need to be in tables. IMHO, where we bring more consistency and simplicity to our markup, and that results in the page view looking bad, we need to ask whether the page layout is actually being handled correctly. Obviously it means even more work... and if there is someone in particular maintaining that mod that person should be involved. But do we want to establish some CSS guidelines and then revisit this with those persons in on the conversation? What's your opinion?
            Hide
            lazydaisy Mary Evans added a comment -

            I am inclined to go with your suggestion, Amy, and wait until we have some better guidelines in place and then revising this, and any other areas that needs attention, with the developers who maintain them.

            Luckily enough, this is the only tracker issue that has been queried, which accounts for 10 tables, so this means just over 2/3rds have been been passed.

            Show
            lazydaisy Mary Evans added a comment - I am inclined to go with your suggestion, Amy, and wait until we have some better guidelines in place and then revising this, and any other areas that needs attention, with the developers who maintain them. Luckily enough, this is the only tracker issue that has been queried, which accounts for 10 tables, so this means just over 2/3rds have been been passed.
            Hide
            lazydaisy Mary Evans added a comment - - edited

            @ Marina Glancy

            Thanks for testing this Marina. I agree the pages look better without the line.
            I am just wondering if we could achieve the same look using a grid system for the layout. I'll have to look into this.

            Cheers
            Mary

            Show
            lazydaisy Mary Evans added a comment - - edited @ Marina Glancy Thanks for testing this Marina. I agree the pages look better without the line. I am just wondering if we could achieve the same look using a grid system for the layout. I'll have to look into this. Cheers Mary
            Hide
            poltawski Dan Poltawski added a comment -

            Hi,

            We're at the end of the integration cycle for this week and a fix hasn't arrived so i'm going to have to revert this.

            Hopefully wecan get this sorted for next week. Thanks!

            Show
            poltawski Dan Poltawski added a comment - Hi, We're at the end of the integration cycle for this week and a fix hasn't arrived so i'm going to have to revert this. Hopefully wecan get this sorted for next week. Thanks!
            Hide
            lazydaisy Mary Evans added a comment -

            Thanks Dan. Closing this as deferred.

            Show
            lazydaisy Mary Evans added a comment - Thanks Dan. Closing this as deferred.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/13