{"action":"created","issue":{"url":"https://api.github.com/repos/edx/edx-platform/issues/12419","repository_url":"https://api.github.com/repos/edx/edx-platform","labels_url":"https://api.github.com/repos/edx/edx-platform/issues/12419/labels{/name}","comments_url":"https://api.github.com/repos/edx/edx-platform/issues/12419/comments","events_url":"https://api.github.com/repos/edx/edx-platform/issues/12419/events","html_url":"https://github.com/edx/edx-platform/pull/12419","id":154081647,"number":12419,"title":"AC-454 adding visual clarity for selected menu options in the video player","user":{"login":"clrux","id":2112024,"avatar_url":"https://avatars.githubusercontent.com/u/2112024?v=3","gravatar_id":"","url":"https://api.github.com/users/clrux","html_url":"https://github.com/clrux","followers_url":"https://api.github.com/users/clrux/followers","following_url":"https://api.github.com/users/clrux/following{/other_user}","gists_url":"https://api.github.com/users/clrux/gists{/gist_id}","starred_url":"https://api.github.com/users/clrux/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/clrux/subscriptions","organizations_url":"https://api.github.com/users/clrux/orgs","repos_url":"https://api.github.com/users/clrux/repos","events_url":"https://api.github.com/users/clrux/events{/privacy}","received_events_url":"https://api.github.com/users/clrux/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2016-05-10T19:01:35Z","updated_at":"2016-05-16T20:27:19Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/edx/edx-platform/pulls/12419","html_url":"https://github.com/edx/edx-platform/pull/12419","diff_url":"https://github.com/edx/edx-platform/pull/12419.diff","patch_url":"https://github.com/edx/edx-platform/pull/12419.patch"},"body":"# [AC-454](https://openedx.atlassian.net/browse/AC-454)\r\n\r\nThis piece of work adds additional visual cues to selected menu options in the video player. Previously we were only using color to indicate selected options, but we need at least two identifying factors for good accessibility, especially in high contrast modes.\r\n\r\nIn addition to the color shift, I've also bolded the selected choice and placed a small border on the side of the option so it's exceedingly clear which option is chosen. The border is a redundant cue in case the bolder type or color shift isn't enough.\r\n\r\nI also synced up behavior for the two menus (speed and language). Previously a user could change the transcript languages, but the menu would remain open, whereas choosing an option in the speed would update the selection and close the menu. Now both menus close on choice selection.\r\n\r\nTests already check for the presence of `is-active` on both of these menus.\r\n\r\n## Visual updates\r\n\r\n![screen shot 2016-05-10 at 2 57 31 pm](https://cloud.githubusercontent.com/assets/2112024/15158923/0bbdcd5a-16c0-11e6-9d74-f0c961b294ff.png)\r\n_Normal display_\r\n\r\n![img_20160511_082617](https://cloud.githubusercontent.com/assets/2112024/15180900/3ea1a13c-1752-11e6-902b-2ff2c7544968.jpg)\r\n_High contrast display_\r\n\r\n## Sandbox\r\n\r\nhttps://clrux-ac-454.sandbox.edx.org/courses/course-v1:edX+Justice101+2016_T2/courseware/C_04/lecture_04/\r\n\r\n## Reviewers\r\n\r\n- [ ] @cptvitamin or @clytwynec \r\n- [ ] Someone from Lahore TNL team @muzaffaryousaf is lead"},"comment":{"url":"https://api.github.com/repos/edx/edx-platform/issues/comments/219537378","html_url":"https://github.com/edx/edx-platform/pull/12419#issuecomment-219537378","issue_url":"https://api.github.com/repos/edx/edx-platform/issues/12419","id":219537378,"user":{"login":"cptvitamin","id":444368,"avatar_url":"https://avatars.githubusercontent.com/u/444368?v=3","gravatar_id":"","url":"https://api.github.com/users/cptvitamin","html_url":"https://github.com/cptvitamin","followers_url":"https://api.github.com/users/cptvitamin/followers","following_url":"https://api.github.com/users/cptvitamin/following{/other_user}","gists_url":"https://api.github.com/users/cptvitamin/gists{/gist_id}","starred_url":"https://api.github.com/users/cptvitamin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/cptvitamin/subscriptions","organizations_url":"https://api.github.com/users/cptvitamin/orgs","repos_url":"https://api.github.com/users/cptvitamin/repos","events_url":"https://api.github.com/users/cptvitamin/events{/privacy}","received_events_url":"https://api.github.com/users/cptvitamin/received_events","type":"User","site_admin":false},"created_at":"2016-05-16T20:27:19Z","updated_at":"2016-05-16T20:27:19Z","body":"@clrux this functions as expected in regular and high contrast mode.  I think it makes a big improvement in UX, too.  Making this more accessible to low vision users may be aware of something we should be doing for non-visual users as well, and that is indicating what the current setting for these two controls is, and also indicating that (using screen reader text) in the menu itself.  Can you squeeze that into this PR?  \r\n\r\n1. Looking at the src code I would change the approach for adding the screen reader help text to offscreen text instead of aria-label, that way you can just add a new span with the help text to the button: \"Speed: 1.50x Press up to enter the speed...\"\r\n2. Add screen reader text to the currently selected menu item (class=\"is-active\") that indicates it is the currently selected menu item: \"1.50x selected\""}}