Skip to content

fix(create-grid): correctly compute stack order for non-positioned stacking contexts#3930

Merged
straker merged 8 commits intodevelopfrom
stack-opacity
Mar 15, 2023
Merged

fix(create-grid): correctly compute stack order for non-positioned stacking contexts#3930
straker merged 8 commits intodevelopfrom
stack-opacity

Conversation

@straker
Copy link
Copy Markdown
Contributor

@straker straker commented Mar 3, 2023

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

@straker straker requested a review from a team as a code owner March 3, 2023 00:52
Copy link
Copy Markdown
Contributor

@WilcoFiers WilcoFiers left a comment

Choose a reason for hiding this comment

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

  1. 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

  2. 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
@straker
Copy link
Copy Markdown
Contributor Author

straker commented Mar 6, 2023

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.

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 getElementStack since those aren't fragile and won't break when we update stacking numbers.

Copy link
Copy Markdown
Contributor

@WilcoFiers WilcoFiers left a 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

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.

Color-contrast issue with element stack

2 participants