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.

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

Added ARIA: TableAria #18 and CellAria #14 #26

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Added ARIA: TableAria #18 and CellAria #14 #26

wants to merge 8 commits into from

Conversation

Amishakumari544
Copy link

@Amishakumari544 Amishakumari544 commented Jul 16, 2021

What Changed

Readme File with TableAria Content
Readme File with CellAria Content

Added Table Role
Added Cell Role

Why

This PR is in reference to #14 and #18 which required adding Table and Cell accessibility feature.

"width=''",
"bgcolor=''",
],
"description": "Add `role='radio'` to a checkable interactive control. Use radio in place of checkbox if only one item in a group can be checked. Add `aria-checked` to indicate the state of the checkbox."
Copy link
Collaborator

Choose a reason for hiding this comment

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

whoops copy/paster error!

"TableAria",
],
"body": [
"aria-colcount=''",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you forgot role=table?

Copy link
Author

Choose a reason for hiding this comment

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

except this all things are okay??

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kendallgassner kendallgassner added the minor Increment the minor version when merged label Jul 16, 2021
@Amishakumari544 Amishakumari544 changed the title added ARIA: TableAria #18 Added ARIA: TableAria #18 and CellAria #14 Jul 17, 2021
@Amishakumari544
Copy link
Author

@kendallgassner mam please review this if it is fine or not.

@@ -0,0 +1,6621 @@
{
"name": "accessibility-snippets",
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you remove this file :) we use yarn.lock

Copy link
Collaborator

Choose a reason for hiding this comment

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

also this should not be included: accessibility-snippets-0.0.15.vsix

would be awesome if you added these to the gitignore

"TableAria",
],
"body": [
"aria-colcount=''",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"CellAria",
],
"body": [
"role='cell'",
Copy link
Collaborator

Choose a reason for hiding this comment

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

perfection!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry one thing i forgot to mention you shouldnt use bot the quotes and the brackets at the same time
quotes are for string values like aria-label for example.
and {} are for things like numbers

variable={$1} => variable={}
variableString='$1' => variable=''

Copy link
Collaborator

@kendallgassner kendallgassner left a comment

Choose a reason for hiding this comment

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

looks like you forgot to remove accessibility-snippets-0.0.15.vsix

"role='table'",
"aria-colcount='{$1}'",
"aria-rowcount={-1}",
"aria-label ={}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this needs $2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants