Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: HTML and CSS, Libraries
    • Labels:
    • Testing Instructions:
      Hide

      Having the totara elementlibrary installed helps as it has a page which excercises the tab render in a variety of ways.

      If not then visit anything with tabs, Wiki, Lesson, Quiz editing, Glossary.

      Check

      a) the HTML produced is in line with that documented by Bootstrap:

      http://twitter.github.io/bootstrap/components.html#navs

      b) the visual appearance is consistent with above, and doesn't clash with surroundings in Moodle Bootstrapbase theme

      c) tabs with a second level should render as one row above the other

      d) the tabs should collapse into what Bootstrap calls stackable tabs at screen widths less than 480px

      Show
      Having the totara elementlibrary installed helps as it has a page which excercises the tab render in a variety of ways. If not then visit anything with tabs, Wiki, Lesson, Quiz editing, Glossary. Check a) the HTML produced is in line with that documented by Bootstrap: http://twitter.github.io/bootstrap/components.html#navs b) the visual appearance is consistent with above, and doesn't clash with surroundings in Moodle Bootstrapbase theme c) tabs with a second level should render as one row above the other d) the tabs should collapse into what Bootstrap calls stackable tabs at screen widths less than 480px
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-39388-master
    • Rank:
      50025

      Description

      The code for generating tabs got moved to a renderer. This is a renderer for the Bootstrapbase theme that generates the standard Bootstrap HTML for tabs as documented here:

      http://twitter.github.io/bootstrap/components.html#navs

      As a result we can also delete some CSS that was being generated to make Bootstrap's standard tab HTML look superficially like Bootstrap.

        Activity

        Hide
        Marina Glancy added a comment -

        Looks good to me!
        Maybe needs adding phpdocs to the render_ functions (even if it is copy-paste from the parent class)

        Show
        Marina Glancy added a comment - Looks good to me! Maybe needs adding phpdocs to the render_ functions (even if it is copy-paste from the parent class)
        Hide
        David Scotson added a comment -

        I copied in the docs from the parent class.

        I'm happy for this to go to integration if you're okay with it. I don't have the button to do it myself.

        cheers,

        dave

        Show
        David Scotson added a comment - I copied in the docs from the parent class. I'm happy for this to go to integration if you're okay with it. I don't have the button to do it myself. cheers, dave
        Hide
        Marina Glancy added a comment -

        Thank you David. Submitting for integration

        Show
        Marina Glancy added a comment - Thank you David. Submitting for integration
        Hide
        Damyon Wiese added a comment -

        I just checked and David M says this change wont break the behat tests because:

        a) we are just looking for the links in the tabs and
        b) we are not running the behat tests under bootstrap/clean atm.

        Show
        Damyon Wiese added a comment - I just checked and David M says this change wont break the behat tests because: a) we are just looking for the links in the tabs and b) we are not running the behat tests under bootstrap/clean atm.
        Hide
        Damyon Wiese added a comment -

        Thanks David,

        This needs some work though - it is incorrectly escaping url parameters for the tabs which breaks the links. To reproduce the error - just add a feedback module and click on "Show responses".

        Show
        Damyon Wiese added a comment - Thanks David, This needs some work though - it is incorrectly escaping url parameters for the tabs which breaks the links. To reproduce the error - just add a feedback module and click on "Show responses".
        Hide
        Marina Glancy added a comment -

        TO INTEGRATORS: please pick up last commit in
        git://github.com/marinaglancy/moodle.git
        branch
        wip-MDL-39388-master

        Show
        Marina Glancy added a comment - TO INTEGRATORS: please pick up last commit in git://github.com/marinaglancy/moodle.git branch wip- MDL-39388 -master
        Hide
        Damyon Wiese added a comment -

        Thanks David and Marina (especially for the quick fix!).

        This has been integrated to master now.

        Show
        Damyon Wiese added a comment - Thanks David and Marina (especially for the quick fix!). This has been integrated to master now.
        Hide
        Mark Nelson added a comment -

        One thing I noticed was that in the bootstrap examples the href is set to "#" for active links. I don't think this really matters though and I imagine the anchor tag is only placed around active text for CSS purposes. So,looks good to me. Thanks for your contribution. Passing.

        Show
        Mark Nelson added a comment - One thing I noticed was that in the bootstrap examples the href is set to "#" for active links. I don't think this really matters though and I imagine the anchor tag is only placed around active text for CSS purposes. So,looks good to me. Thanks for your contribution. Passing.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Did you think this day was not going to arrive ever?

        Your patience has been rewarded, yay, sent upstream, thanks!

        Closing...ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: