Skip to content

Commit

Permalink
[Locked Figure Labels] Add color select to locked figure labels setti…
Browse files Browse the repository at this point in the history
…ngs (#1693)

## Summary:
Add back the color select into the locked figure labels. The content authors
would be able to use the color select to specify colors separately from the
figure color itself.

Charlie made a good point:
> I like having the dropdown option. I imagine it would primarily be useful in the case where a label overlaps another figure, where we might need to choose a color based on having adequate color contrast with that other figure. (I hope that case would be infrequent, since it would also introduce labeling ambiguities for sighted users.)

Issue: https://khanacademy.atlassian.net/browse/LEMS-2431

## Test plan:
`yarn jest`

Storybook
- Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-figure-labels-all-flags
- Check that all the locked labels have color selects
- Confirm that the color select changes the color
- Confirm that changing the overall figure's color select still overrides the label color


<img width="389" alt="Screenshot 2024-09-27 at 3 35 05 PM" src="https://github.com/user-attachments/assets/84c7ec2d-2e52-4e3d-9299-1045cd220698">

Author: nishasy

Reviewers: anakaren-rojas, #perseus, benchristel, catandthemachines

Required Reviewers:

Approved By: anakaren-rojas

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

Pull Request URL: #1693
  • Loading branch information
nishasy authored Sep 30, 2024
1 parent fc16bc7 commit 466d010
Show file tree
Hide file tree
Showing 9 changed files with 21 additions and 33 deletions.
6 changes: 6 additions & 0 deletions .changeset/purple-numbers-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

[Locked Figure Labels] Add color select to locked figure labels settings
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ describe("LockedEllipseSettings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ describe("Locked Function Settings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,6 @@ describe("Locked Label Settings", () => {
expect(titleText).toBeInTheDocument();
});

test("does not show color select when rendered within a locked figure (no onMove)", () => {
// Arrange
render(<LockedLabelSettings {...defaultProps} onMove={undefined} />, {
wrapper: RenderStateRoot,
});

// Act
const colorSwitch = screen.queryByLabelText("color");

// Assert
expect(colorSwitch).not.toBeInTheDocument();
});

describe("Header interactions", () => {
test("should show the correct coords in the header", () => {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,19 +130,14 @@ export default function LockedLabelSettings(props: Props) {
</LabelMedium>

<View style={styles.row}>
{/* Color settings */}
{onMove && (
<>
<ColorSelect
selectedValue={color}
onChange={(newColor: LockedFigureColor) => {
onChangeProps({color: newColor});
}}
style={styles.spaceUnder}
/>
<Strut size={spacing.medium_16} />
</>
)}
<ColorSelect
selectedValue={color}
onChange={(newColor: LockedFigureColor) => {
onChangeProps({color: newColor});
}}
style={styles.spaceUnder}
/>
<Strut size={spacing.medium_16} />

{/* Size settings */}
<LabelMedium tag="label" style={styles.row}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ describe("LockedLineSettings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down Expand Up @@ -424,7 +424,7 @@ describe("LockedLineSettings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ describe("LockedPointSettings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
await userEvent.click(screen.getByText("blue"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ describe("LockedPolygonSettings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ describe("Locked Vector Settings", () => {
);

// Act
const colorSelect = screen.getByLabelText("color");
const colorSelect = screen.getAllByLabelText("color")[0];
await userEvent.click(colorSelect);
const colorOption = screen.getByText("pink");
await userEvent.click(colorOption);
Expand Down

0 comments on commit 466d010

Please sign in to comment.