Skip to content

Conversation

@nwanduka
Copy link
Contributor

Summary

This PR fixes #981

Motivation

Testing

Questions

@birm
Copy link
Member

birm commented Jul 17, 2024

I don't seem to actually see the aria labels on the rendered toolbar items.

@nwanduka
Copy link
Contributor Author

I don't think I understand what you mean. Are you saying you can't "see" the aria-label or when testing with a screen reader, the aria-label isn't called out?

Also could you tell me how you're viewing the changes made? I'd like to be able to confirm that any changes I made work as intended before creating a PR. I don't know how to. Or you could point me to a resource material. Any would be helpful.

@birm birm force-pushed the toolbar-icons-name-change branch from c5eb67c to 6dafdf6 Compare July 19, 2024 15:10
@birm
Copy link
Member

birm commented Jul 19, 2024

The issue that I noticed is that the html for the toolbar icons wasn't including the aria label. I think I've added it in 6dafdf6; please let me know if you agree or not :)

@nwanduka
Copy link
Contributor Author

Ohhhkay. I get it. I missed that. Thanks for pointing it out. And fixing it 😊

@birm
Copy link
Member

birm commented Jul 19, 2024

Does it look ok to merge to you?

@nwanduka
Copy link
Contributor Author

Yes, looks good to me. Ready to merge.

@birm birm merged commit 1841845 into camicroscope:develop Jul 21, 2024
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.

2 participants