-
Notifications
You must be signed in to change notification settings - Fork 779
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
fix(create-grid): correctly compute stack order for non-positioned stacking contexts #3930
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I tried to refactor this function a bit, in order to help myself understand it better, please use that: https://github.com/dequelabs/axe-core/pull/3932/files
-
I think we should move getStackingOrder into its own file. It's only tangentially related to creating the grid. I think separating these two could make things a little clearer.
Overall, I can't seem to keep this all straight in my head. I hope I didn't miss anything, but I'm not sure.
* chore: Refactor createStackingOrder * Remove magic numbers
I don't think we should. The reason being that if we did we'd have to add tests for it, which means testing stacking context arrays. Those would be a pain to maintain as doing something like we just did (refactoring magic numbers, adding a new stack where one wasn't before) would break all the tests. I'd much rather test actual stack results like we are doing in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would have preferred to see createStackingOrder move into its own file, but
While working on the code, I realized that the spec said that if any element that creates a stacking context it would remove the fake stacks created from float or positioned elements that use auto or 0 z-index. So I was able to merge those functions into a single if statement. I also simplified the loop to remove fake stacks so it only needs to look through the array and
splice
once (since any new stack removes all previous fake stacks).Closes: #3929