Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Review long description disclosure example #264

Closed
3 tasks done
mcking65 opened this issue Jan 30, 2017 · 14 comments
Closed
3 tasks done

Review long description disclosure example #264

mcking65 opened this issue Jan 30, 2017 · 14 comments
Assignees
@jongund @jnurthen @annabbott
Labels
Example Page Related to a page containing an example implementation of a pattern
Milestone
Jan 2017 Clean Up

Comments

@mcking65
Copy link
Contributor

mcking65 commented Jan 30, 2017 •
edited
Loading

The example of a
disclosure for an image long description
Is ready for review.

Requested reviews
@mcking65 mcking65 changed the title Review disclosure examples Review long description disclosure example Jan 31, 2017
@mcking65 mcking65 mentioned this issue Jan 31, 2017
Closed
@mcking65 mcking65 added Example Page Related to a page containing an example implementation of a pattern Needs Review labels Jan 31, 2017
@mcking65 mcking65 added this to the Jan 2017 Clean Up milestone Jan 31, 2017
@annabbott
Copy link

Looks good.

@jnurthen
Copy link
Member

jnurthen commented Feb 3, 2017 •
edited
Loading

Good for me. I love the example - I have always loved the amount of information that that chart shows.

@jongund
Copy link
Contributor

jongund commented Feb 13, 2017 •
edited
Loading

Created pull request to with updated visual disclosure indicator to use an image instead of entity value

@mcking65
Copy link
Contributor Author

@jongund, @jnurthen, @annabbott

I have some more questions and possible issues.

  1. Because the image uses aria-labelledby, Screen readers read the entire label twice. It seems to me that the visible text is not really an image label, but is instead a caption. I think the image label should be shorter and represent the nature of the image. This makes me wonder if this shouldn't be a figure with a figcaption?
  2. Is this image a graphical representation of some kind? I am guessing that is not a table. If it is a graph of some kind, maybe it should have aria-label="Chart Detailing Napoleon’s 1812 Russian campaign".
  3. The disclosure is labeled details. But, it exposes a table of data. Seems like a better label would be "Chart Data".

Thoughts?

@jongund
Copy link
Contributor

jongund commented Feb 14, 2017 via email

mcking65 pushed a commit that referenced this issue Feb 14, 2017
@jongund @mcking65
Change Disclosure Examples to Use DL Semantics and Revise CSS
db57547 
Per discussion in issue #264 and #265 for review of disclosure examples, made the following changes.

1. Use CSS generated images instead of entities for visual state information
2. For disclosure controls, use  HTML button element instead of span elements with role=button
3. In FAQ, moved answers inside of a `p` element inside of the `dd` element.
4. Fixed call to method for displaying html source.
5. Revised title and H1 to include the word "Example".
6. Change heading inside long description example to H3 from H2
7. Fix typos
8. Add accessibility features section describing use of CSS.
@annabbott
Copy link

In the Role, Property, State, and Tabindex Attributes section for aria-expanded="false":
CSS attribute selectors (e.g. [aria-expanded="false"]) synchronize the visual states with the value of the aria-expanded attribute.
Suggested edit to match noun/verb usage:
CSS attribute selector (e.g. [aria-expanded="false"]) synchronizes the visual states with the value of the aria-expanded attribute.

Same section for aria-expanded="true":
CSS attribute selectors (e.g. [aria-expanded="true"]) synchronize the visual states with the value of the aria-expanded attribute.
Suggested edit to match noun/verb usage:
CSS attribute selector (e.g. [aria-expanded="true"]) synchronizes the visual states with the value of the aria-expanded attribute.

@annabbott
Copy link

In the Keyboard Support section:
Space or
Enter
Activates the disclosure button, which toggles the visibility of the long description.

Only Enter toggles the visibility. Space blinks the visibility, but doesn't leave it open in order to read.

Using FF and keyboard only to navigate.

mcking65 pushed a commit that referenced this issue Apr 16, 2017
@jongund @mcking65
Image Description Disclosure Example: Revised alt and figcaption text…
383f192 
… and left align figcaption (pull #315)

Made the following updates for issue #264.

1. Made the alt text and figcaption text different so the alt text is brief and the caption is more detailed:
    * Revise alt text to be shorter.
    * Revise figcaption text to include english translation of text that is in the figure.
2. To be consistent with HTML requirements for figcaption, made `figcaption` last element in `figure`.
    * Structured text in figcaption element with `<p>` elements.
    * Moved disclosure button and disclosure content inside of the `<figcaption>` element.
3. Updated CSS to improve styling and left align the caption.
@mcking65
Copy link
Contributor Author

@jongund, @annabbott, @jnurthen, I think the team should take one final look at this before we wrap up the task force review process of this one.

@annabbott
Copy link

Typo in the heading above the table::
Currently is:
"Data for Charles Minars's Chart of Naploean's Invasion of Russia"
Corrected typo::
"Data for Charles Minars's Chart of Napolean's Invasion of Russia"

@annabbott
Copy link

I'm totally confused! I thought the long description was to be a translation of the French included in the image. I don't see any long description - just the table that follows the image.

@annabbott
Copy link

Also, there is no alt text for the image.

mcking65 added a commit that referenced this issue Apr 20, 2017
@mcking65
Long Description Disclosure Example: Correct Typos in Disclosure H3
faad012 
For issue #264, per feedback from @annabbott, modified examples/disclosure/disclosure-img-long-description.html:
Corrected two typos in the H3 contained inside the disclosure.
@mcking65
Copy link
Contributor Author

@annabbott, I wonder what you are looking at. There is definitely alt text and the the figcaption includes the translation of the French from wikipedia.

I fixed the typos. Please have another look.

@annabbott
Copy link

Ok, the example page is entirely different than it was 3 days ago! Now I see the alt text, the figcaption plus the edit you did for the typo. Looks good now.

mcking65 added a commit that referenced this issue Apr 21, 2017
@mcking65
Image Description Disclosure Example: Task Force Review Process Compl…
e12e7ae 
…eted

The Authoring Practices Task Force review process for this example page is now complete.

Modified examples/disclosure/disclosure-img-long-description.html to remove link to review issue #264.
@mcking65
Copy link
Contributor Author

The task force review process for this example is now complete. Thank you Team!! Closing this issue.

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

@jongund jongund

@jnurthen jnurthen

@annabbott annabbott

Labels
Example Page Related to a page containing an example implementation of a pattern
Projects
Command, Toggle, Menu, and Disclosure Button Patterns and Examples Development Project
Status: Complete
Milestone
Jan 2017 Clean Up
Development

No branches or pull requests
4 participants
@jongund @jnurthen @mcking65 @annabbott