Details

    • Type: Sub-task
    • Status: Closed
    • Priority: 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

      Description

      Book activity icon issues

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            fred 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
            fred 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
            fred Frédéric Massart added a comment -

            What about:

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

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

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

            Bah, was just about to duplicate this.

            Show
            poltawski Dan Poltawski added a comment - Bah, was just about to duplicate this.
            Hide
            fred 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
            fred 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
            jfilip 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
            jfilip 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
            fred 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
            fred 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
            poltawski 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
            poltawski 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
            poltawski Dan Poltawski added a comment -

            Integrated, thanks fred.

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

            Simply amazing. I am speechless. Passing.

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

            Y E S !

            Closing as fixed, many thanks!

            Show
            stronk7 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:
                  Fix Release Date:
                  3/Dec/12