Details

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

      Open a form with the editor collapse option available. A True/false question type for example. The feedback fields have the collapse option.

      Hover the mouse over the ‘Show editing tools’ bar. The mouse curser should change to a point, the text should turn red and be underlined.

      Show
      Open a form with the editor collapse option available. A True/false question type for example. The feedback fields have the collapse option. Hover the mouse over the ‘Show editing tools’ bar. The mouse curser should change to a point, the text should turn red and be underlined.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:

      Description

      When you mouse-over the Show/Hide editor tools tab, there needs to be some affordance that it is clickable.

      We certainly need to change the mouse pointer,

      I suggest that we also make the text red and underlined, like hovered links. In fact, adding href="#" might be a way to achieve that. (It needs to look a hovered link in the current theme, whatever that is. I think the unhovered state should probably stay as black text.)

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            colchambers Colin Chambers added a comment -

            Just a css fix. Comments welcome.

            Show
            colchambers Colin Chambers added a comment - Just a css fix. Comments welcome.
            Hide
            timhunt Tim Hunt added a comment -

            Just going to update the issue state with what Colin and I discussed verbally.

            Show
            timhunt Tim Hunt added a comment - Just going to update the issue state with what Colin and I discussed verbally.
            Hide
            timhunt Tim Hunt added a comment -

            So, another related problem is accessibility. You cannot expand/collapse the editor using the keyboard.

            Col are going to try to fix that too, by adding href="#" onto the <a>, so it becomes a tab stop, and then cleaning stuff up until it works.

            Show
            timhunt Tim Hunt added a comment - So, another related problem is accessibility. You cannot expand/collapse the editor using the keyboard. Col are going to try to fix that too, by adding href="#" onto the <a>, so it becomes a tab stop, and then cleaning stuff up until it works.
            Hide
            fred Frédéric Massart added a comment - - edited

            I was going to point out the accessibility issue but Tim just did. Also, while adding the "href" to the <a> tag, it might be worth adding the events on the space bar as well and defining the role of the <a> as a button. Also adding the aria-attribute... (see the expand/collapse, show more/less, section legends and MDL-38716).

            To summarise:

            1. <a> should have:
              • an href attribute
              • a role attribute set to button
              • an aria-controls attribute which value would be the IDs of both hidden elements
              • an event associated to the space key, which is the default trigger for buttons, but keep the <enter> one for ease of use
            2. Hidden elements should have:
              • an aria-live set to polite

            On a side note, I wonder if it'd be useful to move the "Hide editing tools" to a button in TinyMCE tools. I don't think it's likely that users will collapse the editor, but they are likely to expand it when it's collapsed by default. Moving to a button in TinyMCE allows for a more vertical space, and seems to be a good emplacement as the "Fullscreen" and "View HTML" are there as well (appearance options).

            (Trivial: the indentation size should be set to 4 spaces)

            Show
            fred Frédéric Massart added a comment - - edited I was going to point out the accessibility issue but Tim just did. Also, while adding the "href" to the <a> tag, it might be worth adding the events on the space bar as well and defining the role of the <a> as a button. Also adding the aria-attribute... (see the expand/collapse, show more/less, section legends and MDL-38716 ). To summarise: <a> should have: an href attribute a role attribute set to button an aria-controls attribute which value would be the IDs of both hidden elements an event associated to the space key, which is the default trigger for buttons, but keep the <enter> one for ease of use Hidden elements should have: an aria-live set to polite On a side note, I wonder if it'd be useful to move the "Hide editing tools" to a button in TinyMCE tools. I don't think it's likely that users will collapse the editor, but they are likely to expand it when it's collapsed by default. Moving to a button in TinyMCE allows for a more vertical space, and seems to be a good emplacement as the "Fullscreen" and "View HTML" are there as well (appearance options). (Trivial: the indentation size should be set to 4 spaces)
            Hide
            colchambers Colin Chambers added a comment -

            Keyboard accessibility is a separate issue I've logged as MDL-39350. Great spot. Something we need to sort. I added the related comments to it. Thanks

            This specific issue is a simple css fix. I fixed the white space, rebased on latest master and pushed.

            Show
            colchambers Colin Chambers added a comment - Keyboard accessibility is a separate issue I've logged as MDL-39350 . Great spot. Something we need to sort. I added the related comments to it. Thanks This specific issue is a simple css fix. I fixed the white space, rebased on latest master and pushed.
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Colin.

            Show
            timhunt Tim Hunt added a comment - Thanks Colin.
            Hide
            damyon Damyon Wiese added a comment -

            Hi Tim/Colin.

            The change is good - but why is the font colour changed and why red? That makes no sense to me - can we have this patch without the colour change?

            Show
            damyon Damyon Wiese added a comment - Hi Tim/Colin. The change is good - but why is the font colour changed and why red? That makes no sense to me - can we have this patch without the colour change?
            Hide
            timhunt Tim Hunt added a comment -

            The red was to match the styling of the exapand/collapse section link.

            Show
            timhunt Tim Hunt added a comment - The red was to match the styling of the exapand/collapse section link.
            Hide
            damyon Damyon Wiese added a comment -

            Ah - now I see it everywhere (sorry should have checked harder)

            Thanks Tim

            Show
            damyon Damyon Wiese added a comment - Ah - now I see it everywhere (sorry should have checked harder) Thanks Tim
            Hide
            damyon Damyon Wiese added a comment -

            Thanks Tim and Colin,

            This has been integrated to 25.

            Show
            damyon Damyon Wiese added a comment - Thanks Tim and Colin, This has been integrated to 25.
            Hide
            abgreeve Adrian Greeve added a comment -

            Works as described.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - Works as described. Test passed.
            Hide
            stronk7 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
            stronk7 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:
                5 Start watching this issue

                Dates

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