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

          Attachments

            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