Skip to content

Commit

Permalink
Remove z-index tokens, constants, and classes that place SOME widgets…
Browse files Browse the repository at this point in the history
… above the doodlepad. (#2170)

## Summary
I did this in two passes -- one commit to remove `above scratchpad`, and a second for `interactive`, which seems to be used for the same purpose. Both place widgets above the ~scratchpad~ doodlepad, which has long led to many bugs.
- **remove all references to "above scratchpad"**
- **remove z-index from more widgets**

Issue: TUT-1059

Author: nedredmond

Reviewers: nedredmond, SonicScrewdriver, jeremywiebe, catandthemachines, mark-fitzgerald, MikeKlemarewski

Required Reviewers:

Approved By: SonicScrewdriver

Checks: ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x)

Pull Request URL: #2170
  • Loading branch information
nedredmond authored Feb 3, 2025
1 parent dbd4967 commit 7f88f17
Show file tree
Hide file tree
Showing 33 changed files with 125 additions and 231 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-doors-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Removes explicit z-indexes from many widgets to fix long-standing bugs in consuming code.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`graph should render 1`] = `
<div>
<div
class="graphie-container above-scratchpad"
class="graphie-container blank-background"
style="width: 400px; height: 400px;"
>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ exports[`Sortable renders a spinner while waiting for the TeX renderer to load:
class="sortable_13d4756 perseus-sortable"
>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand All @@ -55,7 +55,7 @@ exports[`Sortable renders a spinner while waiting for the TeX renderer to load:
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand All @@ -74,7 +74,7 @@ exports[`Sortable renders a spinner while waiting for the TeX renderer to load:
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px;"
>
<div
Expand Down Expand Up @@ -102,7 +102,7 @@ exports[`Sortable should snapshot: first render 1`] = `
class="sortable_13d4756 perseus-sortable"
>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand All @@ -121,7 +121,7 @@ exports[`Sortable should snapshot: first render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand All @@ -140,7 +140,7 @@ exports[`Sortable should snapshot: first render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px;"
>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ exports[`sorter widget should snapshot on mobile: first mobile render 1`] = `
class="sortable_13d4756 perseus-sortable"
>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 8px 0px 0px;"
>
<div
Expand Down Expand Up @@ -64,7 +64,7 @@ exports[`sorter widget should snapshot on mobile: first mobile render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 8px 0px 0px;"
>
<div
Expand Down Expand Up @@ -94,7 +94,7 @@ exports[`sorter widget should snapshot on mobile: first mobile render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px;"
>
<div
Expand Down Expand Up @@ -166,7 +166,7 @@ exports[`sorter widget should snapshot: first render 1`] = `
class="sortable_13d4756 perseus-sortable"
>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand Down Expand Up @@ -196,7 +196,7 @@ exports[`sorter widget should snapshot: first render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px 5px 0px 0px;"
>
<div
Expand Down Expand Up @@ -226,7 +226,7 @@ exports[`sorter widget should snapshot: first render 1`] = `
</div>
</li>
<li
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-interactive perseus-sortable-draggable"
class="card_1s6cuhy-o_O-draggable_12to336-o_O-horizontalCard_9um937 perseus-sortable-draggable"
style="position: static; margin: 0px;"
>
<div
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/components/graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ class Graph extends React.Component<Props> {

return (
<div
className="graphie-container above-scratchpad"
className="graphie-container blank-background"
style={{
width: this.props.box[0],
height: this.props.box[1],
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/components/input-with-examples.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class InputWithExamples extends React.Component<Props, State> {

_getInputClassName: () => string = () => {
// Otherwise, we need to add these INPUT and FOCUSED tags here.
let className = ApiClassNames.INPUT + " " + ApiClassNames.INTERACTIVE;
let className = ApiClassNames.INPUT;
if (this.state.focused) {
className += " " + ApiClassNames.FOCUSED;
}
Expand Down
4 changes: 1 addition & 3 deletions packages/perseus/src/components/sortable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import ReactDOM from "react-dom";
import _ from "underscore";

import {getDependencies} from "../dependencies";
import {ClassNames as ApiClassNames} from "../perseus-api";
import Renderer from "../renderer";
import Util from "../util";

Expand Down Expand Up @@ -351,8 +350,7 @@ class Draggable extends React.Component<DraggableProps, DraggableState> {
!includePadding && styles.unpaddedCard,
) +
" " +
ApiClassNames.INTERACTIVE +
" perseus-sortable-draggable";
"perseus-sortable-draggable";

if (!includePadding) {
className += " perseus-sortable-draggable-unpadded";
Expand Down
1 change: 0 additions & 1 deletion packages/perseus/src/perseus-api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ export const ClassNames = {
SELECTED: "perseus-radio-selected",
OPTION_CONTENT: "perseus-radio-option-content",
},
INTERACTIVE: "perseus-interactive",
CORRECT: "perseus-correct",
INCORRECT: "perseus-incorrect",
UNANSWERED: "perseus-unanswered",
Expand Down
4 changes: 0 additions & 4 deletions packages/perseus/src/styles/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ export const pureMdMax = "1023px";
export const pureLgMax = "1279px";
// @tableBackgroundAccent: #f9f9f9; // for striping
export const tableBackgroundAccent = "#F9F9F9";
// @zIndexAboveScratchpad: @zIndexScratchPad + 1;
export const zIndexAboveScratchpad = 2;
// @zIndexInteractiveComponent: @zIndexAboveScratchpad + 1;
export const zIndexInteractiveComponent = 3;
// @phoneMargin: 16px;
export const phoneMargin = 16;

Expand Down
22 changes: 0 additions & 22 deletions packages/perseus/src/styles/perseus-renderer.less
Original file line number Diff line number Diff line change
Expand Up @@ -37,33 +37,11 @@
background-color: transparent;
}

.above-scratchpad {
position: relative;
z-index: @zIndexAboveScratchpad;
}

// All graphie components placed above the scratchpad should have their
// background wiped out.
.graphie.above-scratchpad,
.graphie-container.above-scratchpad {
.blank-background;
}

.perseus-mobile .graphie-container.above-scratchpad {
background: #ffffff;
}

// Selectable graphie components make for awkward touch experiences
.graphie {
.no-select;
}

.perseus-interactive,
.perseus-interactive.above-scratchpad {
position: relative;
z-index: @zIndexInteractiveComponent;
}

&, // and moar specificity...
#answercontent input[type=text],
#answercontent input[type=number],
Expand Down
20 changes: 2 additions & 18 deletions packages/perseus/src/styles/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,10 @@ import mediaQueries from "./media-queries";

import type {StyleDeclaration} from "aphrodite";

const {
zIndexAboveScratchpad,
zIndexInteractiveComponent,
radioBorderColor,
checkedColor,
circleSize,
radioMarginWidth,
} = constants;
const {radioBorderColor, checkedColor, circleSize, radioMarginWidth} =
constants;

export default StyleSheet.create({
perseusInteractive: {
zIndex: zIndexInteractiveComponent,
position: "relative",
},

aboveScratchpad: {
position: "relative",
zIndex: zIndexAboveScratchpad,
},

blankBackground: {
// TODO(emily): Use WB colors?
backgroundColor: "#FDFDFD",
Expand Down
8 changes: 3 additions & 5 deletions packages/perseus/src/styles/variables.less
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@
@pure-md-max: (@pure-lg-min - 1);
@pure-lg-max: (@pure-xl-min - 1);

// TODO: Remove these; only handle stacking with DOM order
// Layer variables
@zIndexScratchPad: 1;
@zIndexAboveScratchpad: @zIndexScratchPad + 1;
@zIndexInteractiveComponent: @zIndexAboveScratchpad + 1;
@zIndexCurrentlyDragging: @zIndexInteractiveComponent + 1;
@zIndexCalculator: @zIndexCurrentlyDragging + 1;
@zIndexCurrentlyDragging: 4;
@zIndexCalculator: 5;

// Responsive variables
@phoneMargin: 16px;
Expand Down
4 changes: 0 additions & 4 deletions packages/perseus/src/styles/widgets/categorizer.less
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
table {
min-width: 0;
}

label {
.above-scratchpad;
}
}

// TODO(benkomalo): ughhh. kill or move this out of here :(
Expand Down
1 change: 0 additions & 1 deletion packages/perseus/src/styles/widgets/expression.less
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
color: #fcc335;
cursor: pointer;
font-size: 20px;
.perseus-interactive;
}

.error-text {
Expand Down
1 change: 0 additions & 1 deletion packages/perseus/src/styles/widgets/orderer.less
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
}

.card-wrap {
.perseus-interactive;
width: auto;
}

Expand Down
9 changes: 0 additions & 9 deletions packages/perseus/src/styles/widgets/sortable.less
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,7 @@
box-shadow: 0 1px 2px #ccc;
}

// TODO(jack): Make this work for the orderer too.
// Right now don't do this for the orderer because it uses
// more complicated positioning that this would break
.cards-area {
.above-scratchpad;
}

.card {
.perseus-interactive;

background-color: #fff;
border: 1px solid #b9b9b9;
border-bottom-color: #939393;
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/widget-container.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import ReactDOM from "react-dom";

import {DependenciesContext} from "./dependencies";
import ErrorBoundary from "./error-boundary";
import {zIndexInteractiveComponent} from "./styles/constants";
import {containerSizeClass, getClassFromWidth} from "./util/sizing-utils";
import * as Widgets from "./widgets";

Expand Down Expand Up @@ -127,6 +126,7 @@ class WidgetContainer extends React.Component<Props, State> {
// Hack to prevent interaction with static widgets: we overlay a big
// div on top of the widget and overflow: hidden the container.
// Ideally widgets themselves should know how to prevent interaction.
// UPDATE HTML5: `inert` on the underlying div would be better
const isStatic = this.state.widgetProps.static || apiOptions.readOnly;
const staticContainerStyles = {
position: "relative",
Expand All @@ -138,7 +138,7 @@ class WidgetContainer extends React.Component<Props, State> {
position: "absolute",
top: 0,
left: 0,
zIndex: zIndexInteractiveComponent,
zIndex: 3,
} as const;

// Some widgets may include strings of markdown that we may
Expand Down
Loading

0 comments on commit 7f88f17

Please sign in to comment.