Skip to content
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

Use :focus-visible to show focus indictor on polygon #1458

Conversation

benchristel
Copy link
Member

@benchristel benchristel commented Jul 29, 2024

I felt like suggesting these changes inline on #1455 would be too cumbersome, so I'm doing it in a PR instead.

@benchristel benchristel self-assigned this Jul 29, 2024
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/perseus for changes to packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx, packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

@khan-actions-bot khan-actions-bot requested a review from a team July 29, 2024 20:47
Copy link
Contributor

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (9515cbb) and published it to npm. You
can install it using the tag PR1458.

Example:

yarn add @khanacademy/perseus@PR1458

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1458

Copy link
Contributor

github-actions bot commented Jul 29, 2024

Size Change: -89 B (-0.01%)

Total Size: 850 kB

Filename Size Change
packages/perseus/dist/es/index.js 412 kB -89 B (-0.02%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 38.2 kB
packages/kmath/dist/es/index.js 4.27 kB
packages/math-input/dist/es/index.js 80.6 kB
packages/math-input/dist/es/strings.js 1.73 kB
packages/perseus-core/dist/es/index.js 1.48 kB
packages/perseus-editor/dist/es/index.js 271 kB
packages/perseus-linter/dist/es/index.js 21.6 kB
packages/perseus/dist/es/strings.js 3.21 kB
packages/pure-markdown/dist/es/index.js 3.67 kB
packages/simple-markdown/dist/es/index.js 12.4 kB

compressed-size-action

Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.29%. Comparing base (a8996a1) to head (9515cbb).

Additional details and impacted files
@@                                   Coverage Diff                                    @@
##           tb/LEMS-2200/remove-line-weight-after-moving-polygon    #1458      +/-   ##
========================================================================================
+ Coverage                                                 70.11%   70.29%   +0.18%     
========================================================================================
  Files                                                       508      508              
  Lines                                                    105054   105051       -3     
  Branches                                                  10691    10718      +27     
========================================================================================
+ Hits                                                      73656    73848     +192     
+ Misses                                                    31398    31203     -195     

Impacted file tree graph

Files Coverage Δ
...tive-graphs/graphs/components/angle-indicators.tsx 89.39% <ø> (+1.17%) ⬆️
.../src/widgets/interactive-graphs/graphs/polygon.tsx 90.38% <100.00%> (+1.14%) ⬆️

... and 23 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8996a1...9515cbb. Read the comment docs.

@benchristel benchristel requested review from Myranae and removed request for a team July 29, 2024 21:53
@khan-actions-bot khan-actions-bot requested a review from a team July 29, 2024 21:55
@@ -16,7 +16,6 @@ interface PolygonAngleProps {
centerPoint: vec.Vector2;
endPoints: [vec.Vector2, vec.Vector2];
polygonLines: readonly CollinearTuple[];
active: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

This prop wasn't used. Removing it so we can remove the logic to compute its value in polygon.tsx.

@benchristel
Copy link
Member Author

Tamara incorporated these changes into #1455 manually, so there's no need for this PR to stay open. Closing.

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.

2 participants