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

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

        Activity

        Hide
        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
        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
        Mary Evans added a comment -

        RE-BASED

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

        Nice simple tidy up there thanks Mary.

        Show
        Sam Hemelryk added a comment - Nice simple tidy up there thanks Mary.
        Hide
        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 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 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 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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Mary Evans added a comment -

        Thanks Dan. Closing this as deferred.

        Show
        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: