Skip to content

Commit

Permalink
Fix horizontal stretch for graded group dots, + a11y tweaks (#784)
Browse files Browse the repository at this point in the history
LC-1291

Blocks some global list styles from affecting indicator dot `ul` container. This was causing a horizontal stretch in mobile, as well as other more subtle side effects.
Also, increases clickable area to 24 by 24 and adds hover/focus states. This required some refactoring of the current code. Now, instead of an `li` containing a `span`, it's an `li` containing a WonderBlocks `Clickable` containing a `span`... and when it's active, it contains another span! The interior span is to get around the background filling in the double border.

Style changes per this design:
https://www.figma.com/file/2lUPOSbOP8tbW7RLqbBFLh/Expression-Widget?type=design&node-id=5018-31744&mode=design&t=Ln3lgFaqAagSV97A-0

Author: nedredmond

Reviewers: jeremywiebe, nedredmond

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 16.x), ✅ Cypress (ubuntu-latest, 16.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald

Pull Request URL: #784
  • Loading branch information
nedredmond authored Oct 26, 2023
1 parent aa903ae commit cfff6ad
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 46 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-days-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Graded Group Set indicator dots now have a 24x24 clickable area as well as hover/focus states. Stretching issue on mobile resolved as well.
4 changes: 3 additions & 1 deletion packages/perseus/src/styles/articles.less
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,9 @@
margin-bottom: 2 * @baseUnit;
}

.perseus-renderer > .paragraph ul:not(.perseus-widget-radio) {
.perseus-renderer
> .paragraph
ul:not(.perseus-widget-radio, .indicatorContainer) {
margin: 0 0 0 1em;
padding: 0;

Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/styles/perseus-renderer.less
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@
margin: -11px 0px 22px 0px; // first-level lists need padding
}

.paragraph ul:not(.perseus-widget-radio) {
.paragraph ul:not(.perseus-widget-radio, .indicatorContainer) {
padding-left: 35px;
list-style-type: disc;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,32 +38,63 @@ exports[`graded group widget should snapshot 1`] = `
class="spacer_bc4egv"
/>
<ul
class="indicatorContainer_rjg7c2"
class="indicatorContainer_2pd7o4 indicatorContainer"
>
<li
aria-label="Skip to Problem 1a"
class="indicator_qk5jgn-o_O-selectedIndicator_14jsc6v"
role="button"
tabindex="0"
class="indicator_7k5kza"
>
<span
class="srOnly_19bpjuy"
<button
aria-disabled="false"
aria-label="Skip to Problem 1a"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-indicatorButton_8uaklu"
role="button"
type="button"
>
Current
</span>
<div
class="default_xu2jcg-o_O-indicatorDot_yy8ygm"
>
<div
class="default_xu2jcg-o_O-indicatorDotActive_1pky1a"
>
<span
class="srOnly_19bpjuy"
>
Current
</span>
</div>
</div>
</button>
</li>
<li
aria-label="Skip to Problem 1b"
class="indicator_qk5jgn"
role="button"
tabindex="0"
/>
class="indicator_7k5kza"
>
<button
aria-disabled="false"
aria-label="Skip to Problem 1b"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-indicatorButton_8uaklu"
role="button"
type="button"
>
<div
class="default_xu2jcg-o_O-indicatorDot_yy8ygm"
/>
</button>
</li>
<li
aria-label="Skip to Problem 1c"
class="indicator_qk5jgn"
role="button"
tabindex="0"
/>
class="indicator_7k5kza"
>
<button
aria-disabled="false"
aria-label="Skip to Problem 1c"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-indicatorButton_8uaklu"
role="button"
type="button"
>
<div
class="default_xu2jcg-o_O-indicatorDot_yy8ygm"
/>
</button>
</li>
</ul>
</div>
<div
Expand Down
95 changes: 70 additions & 25 deletions packages/perseus/src/widgets/graded-group-set.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable @khanacademy/ts-no-error-suppressions */
import {linterContextDefault} from "@khanacademy/perseus-linter";
import Clickable from "@khanacademy/wonder-blocks-clickable";
import Color from "@khanacademy/wonder-blocks-color";
import {View} from "@khanacademy/wonder-blocks-core";
import * as i18n from "@khanacademy/wonder-blocks-i18n";
import {StyleSheet, css} from "aphrodite";
import classNames from "classnames";
import * as React from "react";

import {getDependencies} from "../dependencies";
Expand Down Expand Up @@ -38,28 +41,43 @@ class Indicators extends React.Component<IndicatorsProps> {

render(): React.ReactNode {
return (
<ul className={css(styles.indicatorContainer)}>
<ul
// reduntantly add class name for use in .less files--
// the styles object key gets hashed
className={classNames(
css(styles.indicatorContainer),
"indicatorContainer",
)}
>
{this.props.gradedGroups.map(({title}, i) => (
<li
role="button"
aria-label={i18n._("Skip to %(title)s", {
title,
})}
key={title}
className={css(
styles.indicator,
i === this.props.currentGroup &&
styles.selectedIndicator,
)}
tabIndex={0}
onClick={() => this.props.onChangeCurrentGroup(i)}
onKeyDown={(e) => this.handleKeyDown(e, i)}
>
{i === this.props.currentGroup && (
<span className={css(a11y.srOnly)}>
{i18n._("Current")}
</span>
)}
<li className={css(styles.indicator)} key={title}>
<Clickable
role="button"
aria-label={i18n._("Skip to %(title)s", {
title,
})}
style={styles.indicatorButton}
onClick={() => this.props.onChangeCurrentGroup(i)}
onKeyDown={(e) => this.handleKeyDown(e, i)}
>
{({hovered, focused, pressed}) => (
<View
style={[
styles.indicatorDot,
(hovered || focused || pressed) &&
styles.indicatorDotFocused,
]}
>
{i === this.props.currentGroup && (
<View style={styles.indicatorDotActive}>
<span className={css(a11y.srOnly)}>
{i18n._("Current")}
</span>
</View>
)}
</View>
)}
</Clickable>
</li>
))}
</ul>
Expand Down Expand Up @@ -262,20 +280,47 @@ const styles = StyleSheet.create({
flexDirection: "row",
listStyle: "none",
margin: "unset",
paddingInlineStart: "unset",
flexWrap: "wrap",
},

indicator: {
width: 24,
height: 24,
},

indicatorButton: {
width: "100%",
height: "100%",
display: "flex",
flexWrap: "wrap",
placeContent: "center",
cursor: "pointer",

":focus": {
outline: "none",
},
},

indicatorDot: {
boxSizing: "content-box",
width: 10,
height: 10,
borderRadius: "100%",
border: "3px solid",
borderWidth: 2,
borderColor: Color.blue,
marginLeft: 5,
cursor: "pointer",
borderStyle: "solid",
},

indicatorDotFocused: {
borderWidth: 5,
borderStyle: "double",
},

selectedIndicator: {
indicatorDotActive: {
backgroundColor: Color.blue,
width: "100%",
height: "100%",
},

container: {
Expand Down

0 comments on commit cfff6ad

Please sign in to comment.