Details

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

      Test pre-requisites

      • A book module with chapters and sub-chapters

      Test steps

      1. Explore the book as a teacher (editing mode on and off)
      2. Make sure:
        • The icons are nice and displayed
        • The fallback to PNG works
        • RTL is displayed correctly
        • The Table of Content is displayed correctly with any setting for _Chapter formatting _
      Show
      Test pre-requisites A book module with chapters and sub-chapters Test steps Explore the book as a teacher (editing mode on and off) Make sure : The icons are nice and displayed The fallback to PNG works RTL is displayed correctly The Table of Content is displayed correctly with any setting for _Chapter formatting _
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-36628-master
    • Rank:
      46121

      Description

      Book activity icon issues

        Issue Links

          Activity

          Hide
          Frédéric Massart added a comment -

          Add icon in the book block is red. Also padding is not good. See icon_book.png

          Show
          Frédéric Massart added a comment - Add icon in the book block is red. Also padding is not good. See icon_book.png
          Hide
          Frédéric Massart added a comment -

          What about:

          • Generate IMS
          • Print Book
          • Print Chapter
          • The navigation icons?
          Show
          Frédéric Massart added a comment - What about: Generate IMS Print Book Print Chapter The navigation icons?
          Hide
          Barbara Ramiro added a comment -

          Yes book navigation icons plus padding/margin between edit icons on the table of contents

          Show
          Barbara Ramiro added a comment - Yes book navigation icons plus padding/margin between edit icons on the table of contents
          Hide
          Dan Poltawski added a comment -

          Bah, was just about to duplicate this.

          Show
          Dan Poltawski added a comment - Bah, was just about to duplicate this.
          Hide
          Frédéric Massart added a comment -

          The Generate IMS icon has not been worked on yet, it's not as easy as using the Module icon in size 16, it has to be monochrome... Also, I cleaned up a lot the book CSS file which had a lot of repeated rules.

          Show
          Frédéric Massart added a comment - The Generate IMS icon has not been worked on yet, it's not as easy as using the Module icon in size 16, it has to be monochrome... Also, I cleaned up a lot the book CSS file which had a lot of repeated rules.
          Hide
          Justin Filip added a comment - - edited

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

          The code looks good to me. For the Documentation section, I'm not sure if this issue should have the ui_change label applied to it or not. It is a UI change but not one that would require documentation updates, unless someone wanted to take a screenshot with the new icons?

          And also a question: Fred, at one point in the CSS file you modified a line to change something from from margin-left: 0px; to margin-left: 0; and I was curious why that was changed:

          Show
          Justin Filip added a comment - - edited [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check The code looks good to me. For the Documentation section, I'm not sure if this issue should have the ui_change label applied to it or not. It is a UI change but not one that would require documentation updates, unless someone wanted to take a screenshot with the new icons? And also a question: Fred, at one point in the CSS file you modified a line to change something from from margin-left: 0px; to margin-left: 0; and I was curious why that was changed: https://github.com/FMCorz/moodle/compare/020e338...MDL-36628-master#L13L8
          Hide
          Frédéric Massart added a comment -

          Hi Justin, thanks for the review. Interesting point you raised about the ui_change label, I have discussed it and added the label on the meta issue, because most of the issues would need some documentation if we decide to. Also, the 0px to 0 was because my linter annoyed me saying that px is irrelevant when the value is 0. I usually didn't fix it, but here I did apparently.

          Pushing for integration, thanks!

          Show
          Frédéric Massart added a comment - Hi Justin, thanks for the review. Interesting point you raised about the ui_change label, I have discussed it and added the label on the meta issue, because most of the issues would need some documentation if we decide to. Also, the 0px to 0 was because my linter annoyed me saying that px is irrelevant when the value is 0. I usually didn't fix it, but here I did apparently. Pushing for integration, thanks!
          Hide
          Dan Poltawski added a comment -

          Fred: Meta is fine and probably most sensible in this case. But just to be clear, this label isn't just for us. Its so anyone who writes a book, or some training materials can see what changes in each release so they know where they need to update their screenshots etc.

          Show
          Dan Poltawski added a comment - Fred: Meta is fine and probably most sensible in this case. But just to be clear, this label isn't just for us. Its so anyone who writes a book, or some training materials can see what changes in each release so they know where they need to update their screenshots etc.
          Hide
          Dan Poltawski added a comment -

          Integrated, thanks fred.

          Show
          Dan Poltawski added a comment - Integrated, thanks fred.
          Hide
          Mark Nelson added a comment -

          Simply amazing. I am speechless. Passing.

          Show
          Mark Nelson added a comment - Simply amazing. I am speechless. Passing.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Y E S !

          Closing as fixed, many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Y E S ! Closing as fixed, many thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: