Skip to content

Implement minor UI/A11Y fixes for outline - #14859

Merged
bjacobel merged 1 commit into
masterfrom
bjacobel/outline-details
Apr 18, 2017
Merged

Implement minor UI/A11Y fixes for outline#14859
bjacobel merged 1 commit into
masterfrom
bjacobel/outline-details

Conversation

@bjacobel

@bjacobel bjacobel commented Apr 11, 2017

Copy link
Copy Markdown
Contributor

Combined LEARNER-430 and LEARNER-333.

https://outline-details.sandbox.edx.org/courses/course-v1:edX+bj101+2017/course/

  • I would like to add a element on that page (not sure if it should include the -
    Search/Resume/Bookmarks region or not) and make the Topic levels (Introduction, Example Week 1, etc) Headings.
  • There is nothing that screams what this page is to me (Course Outline) so you might consider making that Heading visible.
  • Show Course Outline Title
  • Resume Course uses btn-brand styling
  • Resume Course action is positioned to the right of "Bookmarks", with search still to its right (though disabled on edx.org)
  • Search this Course area should be linked to courseware search feature flag. (may be in separate story?)
  • Apply button border radius to the active "Resume Course" subsection.
  • Fix apostrophe in the search box placeholder text.
  • Fix the vertical alignment of the "Resume Course" button at <400px
  • The title element (in the head) might also be changed to include Course Name Course Outline - Platform Name This isn't possible inside of fragments, wontfix
  • Focus border on section / subsection should not enable based on click (a11y review here as well) Mark S confirmed the only way he knows of to have the focus show for keyboard navigation but not for clicks is with a "super hacky" JavaScript library that we haven't vetted for performance or a11y, so IMO this is also a wontfix

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might as well translate Course Outline here too.

@bjacobel
bjacobel force-pushed the bjacobel/outline-details branch from 75ab248 to 5b51d86 Compare April 11, 2017 17:40
@bjacobel bjacobel changed the title Implement minor UI/A11Y fixes for outline [WIP] Implement minor UI/A11Y fixes for outline Apr 11, 2017
@bjacobel

bjacobel commented Apr 11, 2017

Copy link
Copy Markdown
Contributor Author

Current plan is to wait to merge this until https://github.com/edx/edx-platform/pull/14828 goes to master (I also need to talk to Mark about one of the checklist items before review/sandbox time)

@bjacobel bjacobel changed the title [WIP] Implement minor UI/A11Y fixes for outline Implement minor UI/A11Y fixes for outline Apr 11, 2017
@bjacobel

Copy link
Copy Markdown
Contributor Author

Sandbox is set up, ready for review.

@bjacobel
bjacobel requested a review from andy-armstrong April 11, 2017 20:04

@andy-armstrong andy-armstrong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice set of clean ups! I have one minor comment that you can address if you like.

@@ -1,3 +1,9 @@
.outline-header {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way for you to know this, but the course skeleton defines a page-title class that should be used for consistency:

https://outline-details.sandbox.edx.org/template/ux/reference/course-skeleton.html

@marcotuts

marcotuts commented Apr 12, 2017

Copy link
Copy Markdown
Contributor

I will connect with @cptvitamin around the section / subsection focus prior to merge to discuss how we handled this in the past for discussions.

update: I can connect with mark next week, ok to keep that item as wontfix for now. 👍 from my standpoint.

@bjacobel
bjacobel force-pushed the bjacobel/outline-details branch 4 times, most recently from 5f5cf58 to 1b27390 Compare April 12, 2017 17:17
@dianakhuang
dianakhuang force-pushed the diana/new-transformer branch from 54985eb to 76d68c1 Compare April 12, 2017 17:52
@bjacobel
bjacobel force-pushed the bjacobel/outline-details branch from 1b27390 to 68da899 Compare April 12, 2017 19:23
@dianakhuang
dianakhuang force-pushed the diana/new-transformer branch 3 times, most recently from 8e6c118 to 196d379 Compare April 13, 2017 18:39
@bjacobel
bjacobel changed the base branch from diana/new-transformer to master April 13, 2017 19:57
@bjacobel
bjacobel force-pushed the bjacobel/outline-details branch from 68da899 to 8d56883 Compare April 13, 2017 20:02
@bjacobel
bjacobel force-pushed the bjacobel/outline-details branch from 8d56883 to 89fc9fb Compare April 13, 2017 20:05
@bjacobel

Copy link
Copy Markdown
Contributor Author

jenkins run js

@bjacobel
bjacobel merged commit 447f5c2 into master Apr 18, 2017
@bjacobel
bjacobel deleted the bjacobel/outline-details branch April 18, 2017 14:20
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Wednesday, April 19, 2017.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants