-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add navigation+layout buttons for mobile #7178
Conversation
This reverts commit 1193824.
…ing of viewport meta tag; shrink buttons a bit again
This reverts commit d69a7cf.
…3d viewport is active; fix styling in dark mode
@@ -236,7 +236,8 @@ | |||
"worker-loader": "^3.0.8" | |||
}, | |||
"resolutions": { | |||
"**/mini-store": "^1.1.0" | |||
"**/mini-store": "^1.1.0", | |||
"**/redux": "3.7.2" |
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.
this is not strictly necessary anymore (was a byproduct of trying to get the library use-redux-effect to work). however, I noticed that currently multiple versions of redux are referenced in yarn.lock which doesn't seem ideal (not sure whether they were included in the webpack bundle). so, I thought that pinning the version wouldn't hurt.
frontend/javascripts/oxalis/view/layouting/floating_mobile_controls.tsx
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/layouting/floating_mobile_controls.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Norman Rzepka <[email protected]>
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.
Very cool! I like the simple design and thoughtful UX very much 💯
- Only thing I noticed during testing was that the viewport indicator as well as the forward/backward buttons are not implemented to properly support flight and oblique mode. I think the last active viewport is memorized and the position is adapted according to that.
Depending on if you want to support it, the respective buttons could either be hidden, there, or the viewport button could be hidden and the forward/backward could be adapted to move in the proper w-direction according to the current rotation.
@@ -617,7 +617,7 @@ export class InputMouse { | |||
if (this.lastScale != null) { | |||
const delta = evt.scale - this.lastScale; | |||
this.lastScale = evt.scale; | |||
this.emitter.emit("pinch", 10 * delta); | |||
this.emitter.emit("pinch", 10 * delta, this.position); |
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.
In Slack you mentioned that this 10 *
possibly should be removed
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 meant that the factor 10 feels fishy, but I don't think that it can be simply removed. I suspect that it's there for a reason 🤔 It was added in this PR: https://github.com/scalableminds/webknossos/pull/2070/files#diff-f1af7da1cff58c6ece76e97bdc89036a204b0e0f4ce57a201f5ff8a1b365ef43R455. Since I cannot really test the pinch-to-zoom feature, I'd leave it as is for now..
frontend/javascripts/oxalis/view/layouting/tracing_layout_view.tsx
Outdated
Show resolved
Hide resolved
correct move speed; prevent context menu from opening on long press; automatically blur buttons on touch end
Instead of detecting touch support or mobile devices, I implemented the logic so that the floating buttons appear automatically when the user uses touch as input. Conversely, the buttons are hidden again on mouse over. This should make the UX smooth even for devices where both is possible (e.g., convertible laptops). For narrow screens (se above), the floating bar is shown automatically, but will also disappear on mouse move.
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)