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

some accessibilty changes #359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Rishikesh-microsoft
Copy link
Contributor

  1. Added asterisk symbol for Start and End Date field in DateTimepicker component.
  2. Made save button disabled color as grey color important when input is empty
  3. Changed to a convenient aria-label for zoom-in and zoom-out button AvailabilityChart component.
  4. Changed background color for removing Keros related issues in error messages in DateTimepicker component.
  5. Added role as alert for alert messages in DateTimepicker component..
  6. Added new string constants for asterisk and alert messages in String.ts file.

@ghost
Copy link

ghost commented May 30, 2022

CLA assistant check
All CLA requirements met.

@@ -547,6 +549,9 @@ class DateTimePicker extends ChartComponent{
.attr('for', inputID)
.attr('aria-label', `${startOrEnd === 'start' ? this.getString('Start time input') : this.getString('End time input')}`)
.text(this.getString(startOrEnd));
let timeRequired = timeLabel.append("span")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see you've declared the variable timeRequired here, where are you using it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed the timeRequired variable in my next commit.

@@ -77,9 +77,12 @@ class Strings {
"timezone selection": "timezone selection",
"Start time input": "Start time input",
"End time input": "End time input",
"*": "*",
"snap end time to latest": "snap end time to latest",
"zoom in": "zoom in",
"zoom out": "zoom out",
Copy link
Contributor

@thomasricci thomasricci Jun 2, 2022

Choose a reason for hiding this comment

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

Do you still need these "zoom in" "zoom out"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need this as it is used in title attribute.

@thomasricci
Copy link
Contributor

Do you need to update the library version too ?

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.

3 participants