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

Fix Shift-Click to Deselect Edges/Faces #5289

Merged
merged 29 commits into from
Feb 11, 2025
Merged

Fix Shift-Click to Deselect Edges/Faces #5289

merged 29 commits into from
Feb 11, 2025

Conversation

max-mrgrsk
Copy link
Collaborator

@max-mrgrsk max-mrgrsk commented Feb 6, 2025

Summary

  • Implements proper toggling in the 'Set selection' action so shift-clicking a selected edge/face now removes it from selection instead of duplicating.
  • Adds a Playwright test verifying shift-clicking both single and multiple items toggles them on/off correctly.

Changes

Selections:

In ModelingMachineProvider.tsx under 'Set selection', if shift is pressed, we now check if the item is already selected:

  • If true, remove it (.filter).
  • If false, add it (spread + newItem).

Playwright Test:

Validates selecting/deselecting edges/faces:

  • Single edge → shift-click again → deselect.
  • Multiple edges + face → shift-click each again → deselect all.

Everything now correctly toggles on shift-click as expected.

Closes: #4760

@max-mrgrsk max-mrgrsk linked an issue Feb 6, 2025 that may be closed by this pull request
Copy link

vercel bot commented Feb 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Feb 10, 2025 11:16pm

Copy link

qa-wolf bot commented Feb 6, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@max-mrgrsk max-mrgrsk self-assigned this Feb 6, 2025
@max-mrgrsk max-mrgrsk added the bug Something isn't working label Feb 6, 2025
@max-mrgrsk max-mrgrsk added this to the v1 Modeling App Launch milestone Feb 6, 2025
@max-mrgrsk max-mrgrsk added dev Issues related to development of the app. desktop-app Issues from the desktop app. labels Feb 6, 2025
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.90%. Comparing base (e2be66b) to head (eceb0df).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5289      +/-   ##
==========================================
- Coverage   86.05%   85.90%   -0.15%     
==========================================
  Files          92       92              
  Lines       33300    33495     +195     
==========================================
+ Hits        28656    28774     +118     
- Misses       4644     4721      +77     
Flag Coverage Δ
wasm-lib 85.90% <ø> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@pierremtb pierremtb left a comment

Choose a reason for hiding this comment

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

Looks good, and confirmed it's working locally. Thank you!

@pierremtb pierremtb merged commit 652f82e into main Feb 11, 2025
34 of 35 checks passed
@pierremtb pierremtb deleted the max-edge-deselection branch February 11, 2025 13:16
@franknoirot
Copy link
Collaborator

Great work @max-mrgrsk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop-app Issues from the desktop app. dev Issues related to development of the app.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for edge deselection
3 participants