-
Notifications
You must be signed in to change notification settings - Fork 17
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
Streamline edge selection #498
Conversation
Preview PR at appsharing.space |
Integration tests repot: appsharing.space |
This makes me think we should probably have some UI tests about selecting edges and applying edge operations. Let's do that in a separate PR! |
Yes, that sounds good. |
@@ -103,7 +103,7 @@ export class MainView extends React.Component<IProps, IStates> { | |||
this._mainViewModel.renderSignal.connect(this._requestRender, this); | |||
this._mainViewModel.workerBusy.connect(this._workerBusyHandler, this); | |||
|
|||
this._raycaster.params.Line = { threshold: 50 }; | |||
this._raycaster.params.Line2 = { threshold: 50 }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused by this.
- I see it working properly in Firefox, just like in your screencast. So far so good, but we're not using
Line2
in the codebase anymore, so why does it work? - I do not see it working in Chromium for some reason.
So should we explicitly use Line2
instead of Line
for line meshes in the codebase so that it works in all cases? I would try that, but I really don't understand why it works in Firefox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I do not see it working in Chromium for some reason.
I haven't tested on firefox but I'm testing on Brave and Google Chrome and it both the cases it seems to work as expected on my end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird!
Brave and Google Chrome are both chromium-based browsers. What's wrong with my chromium version 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Let's merge and iterate. I'll see if I can make it work on my chromium locally.
Shall fix #478 & #482
edges.mov