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

Add new option to fix issues with offset dimensions #15

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

Conversation

ExoAndy
Copy link

@ExoAndy ExoAndy commented Jan 30, 2017

Elements which have no offsetWidth and no offsetHeight aren't navigable and therefore it isn't possible to set the focus on those elements even though they're visible to the viewer. To fix that behaviour without breaking anything I have added a new option that will ignore that validation in case it's active and behave as usual in case it isn't active.

Also changed a little the folder structure and added a NPM package configuration file to be able to install Grunt. With the help of Grunt it's quite easy to create minified version of the library now.

Every change was document within the README file.

To be able to add external dependencies like Grunt without adding those to
the repository it's necessary to define a NPM package configuration file.
I added the minified version to the repository to make it available for
everyone who wants to download it without forcing them to clone the
repository and install grunt and all the other dependancies to great the
minified version by their own.
When the library checks whether an element is navigable or not it checks
whether the inner width and height values are greater than zero. There're
cases where the inner width and height are zero but the element is still visible
and therefore needs to be focusable. To fix it I have added a new option
to disable that check for a particular section or globally.
It seems that ignoreInnerDimensionValidation is a more appropriate name for
that option and therefore changed it.
@luke-chang
Copy link
Owner

What amazing patches! Thanks for raising this issue. It'll take me some time to study them. BTW, it would be great if you could provide some code snippets containing visible elements with no innerWidth and innerHeight, so I won't miss any edge case. Thanks a lot.

@ExoAndy
Copy link
Author

ExoAndy commented Feb 1, 2017

You're welcome!
Unfortunately I did a really bad mistake and defined the issue as a innerWidth and innerHeight issue
but what I was really meaning was the offsetWidth and offsetHeight check in the isNavigable function. The feature works as intended just uses the wrong description. I'll fix it asap and also provide a test case for that issue. Sorry about that!

The option name "ignoreInnerDimensionValidation" was wrong because it
doesn't disable the inner dimension validation but the validation of
the offset dimensions. Therefore it was renamed to
"ignoreOffsetDimensionValidation".
@ExoAndy
Copy link
Author

ExoAndy commented Feb 1, 2017

Fixed those issues in the source files. Test cases will follow later.

@ExoAndy ExoAndy changed the title Add new option to fix issues with inner dimensions Add new option to fix issues with offset dimensions Feb 1, 2017
A simple page that shows the different cases where the offset validation
works and when not and how to fix it by using the new option or with
CSS as an alternative way.
@ExoAndy
Copy link
Author

ExoAndy commented Feb 6, 2017

Added the missing test case file to the repository. In that simple HTML file I have added a case where it isn't possible to focus a particular element and also cases where it works by using the new option as well as a solution using CSS.

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