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: 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:
    • Rank:
      49909

      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.)

        Issue Links

          Activity

          Hide
          Colin Chambers added a comment -

          Just a css fix. Comments welcome.

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

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

          Show
          Tim Hunt added a comment - Just going to update the issue state with what Colin and I discussed verbally.
          Hide
          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
          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
          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
          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
          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
          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
          Tim Hunt added a comment -

          Thanks Colin.

          Show
          Tim Hunt added a comment - Thanks Colin.
          Hide
          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 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
          Tim Hunt added a comment -

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

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

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

          Thanks Tim

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

          Thanks Tim and Colin,

          This has been integrated to 25.

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

          Works as described.
          Test passed.

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

              Dates

              • Created:
                Updated:
                Resolved: