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

Haptics #2611

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Haptics #2611

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions android/app/capacitor.build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ android {
apply from: "../capacitor-cordova-android-plugins/cordova.variables.gradle"
dependencies {
implementation project(':capacitor-clipboard')
implementation project(':capacitor-haptics')
implementation project(':capacitor-keyboard')
implementation project(':capacitor-status-bar')

Expand Down
3 changes: 3 additions & 0 deletions android/capacitor.settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ project(':capacitor-android').projectDir = new File('../node_modules/@capacitor/
include ':capacitor-clipboard'
project(':capacitor-clipboard').projectDir = new File('../node_modules/@capacitor/clipboard/android')

include ':capacitor-haptics'
project(':capacitor-haptics').projectDir = new File('../node_modules/@capacitor/haptics/android')

include ':capacitor-keyboard'
project(':capacitor-keyboard').projectDir = new File('../node_modules/@capacitor/keyboard/android')

Expand Down
1 change: 1 addition & 0 deletions ios/App/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ def capacitor_pods
pod 'Capacitor', :path => '../../node_modules/@capacitor/ios'
pod 'CapacitorCordova', :path => '../../node_modules/@capacitor/ios'
pod 'CapacitorClipboard', :path => '../../node_modules/@capacitor/clipboard'
pod 'CapacitorHaptics', :path => '../../node_modules/@capacitor/haptics'
pod 'CapacitorKeyboard', :path => '../../node_modules/@capacitor/keyboard'
pod 'CapacitorStatusBar', :path => '../../node_modules/@capacitor/status-bar'
end
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
"@capacitor/android": "^6.1.2",
"@capacitor/clipboard": "^6.0.2",
"@capacitor/core": "^6.2.0",
"@capacitor/haptics": "^6.0.2",
"@capacitor/ios": "^6.2.0",
"@capacitor/keyboard": "^6.0.2",
"@capacitor/status-bar": "^6.0.1",
Expand Down
5 changes: 5 additions & 0 deletions src/actions/deleteThought.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Capacitor } from '@capacitor/core'
import { Haptics, NotificationType } from '@capacitor/haptics'
import _ from 'lodash'
import Index from '../@types/IndexType'
import Lexeme from '../@types/Lexeme'
Expand Down Expand Up @@ -53,6 +55,9 @@ const deleteThought = (state: State, { local = true, pathParent, thoughtId, orph

// See: Payload.local
const persist = local || remote
if (Capacitor.isNativePlatform()) {
Haptics.notification({ type: NotificationType.Warning })
}
Comment on lines +58 to +60
Copy link
Collaborator

@trevinhofmann trevinhofmann Nov 27, 2024

Choose a reason for hiding this comment

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

One suggestion might be creating our own haptics utility which checks for Capacitor.isNativePlatform() internally so that we don't need to include so many new if blocks throughout the rest of the code.

e.g.:

Suggested change
if (Capacitor.isNativePlatform()) {
Haptics.notification({ type: NotificationType.Warning })
}
haptics.notification({ type: NotificationType.Warning })

A reasonable counterargument is that we want to expressly show within the hooks/actions/components that the haptic event is only triggered on native.

I would be okay with leaving this as it is, but slightly prefer creating a utility.

Copy link
Contributor

Choose a reason for hiding this comment

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

We definitely want to abstract this out since the condition is present with every Haptics call. It should always NOOP on other platforms.

Copy link
Contributor

@raineorshine raineorshine Nov 27, 2024

Choose a reason for hiding this comment

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


// guard against missing lexeme
// while this ideally shouldn't happen, there are some concurrency issues that can cause it to happen, as well as freeThoughts, so we should print an error and just delete the Parent
Expand Down
2 changes: 1 addition & 1 deletion src/components/ActionButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const ActionButton = ({
}),
css({ lineHeight: 2, marginInline: 5, whiteSpace: 'nowrap', fontWeight: 'normal' }),
)}
{...(onClick && !isDisabled ? fastClick(onClick) : null)}
{...(onClick && !isDisabled ? fastClick(onClick, true) : null)}
{...restProps}
>
{/* TODO: Animate on loader toggle. */}
Expand Down
2 changes: 1 addition & 1 deletion src/components/HamburgerMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const HamburgerMenu = () => {
setTimeout(() => {
dispatch(toggleSidebar({}))
}, 10)
})}
}, true)}
>
<Menu width={width} height={width * 0.7} strokeWidth={fontSize / 20} />
</div>
Expand Down
2 changes: 1 addition & 1 deletion src/components/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const Link = React.memo(({ simplePath, label, charLimit = 32, style, cssRaw, cla
setCursor({ path: simplePath }),
toggleSidebar({ value: false }),
])
})}
}, true)}
onMouseDown={e => {
// prevent propagation to Content component which will trigger clickOnEmptySpace
e.stopPropagation()
Expand Down
14 changes: 14 additions & 0 deletions src/components/MultiGesture.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Capacitor } from '@capacitor/core'
import { Haptics, ImpactStyle, NotificationType } from '@capacitor/haptics'
import React, { PropsWithChildren } from 'react'
import { GestureResponderEvent, PanResponder, PanResponderInstance, View } from 'react-native'
import Direction from '../@types/Direction'
Expand Down Expand Up @@ -144,6 +146,9 @@ class MultiGesture extends React.Component<MultiGestureProps> {

// touchcancel is fired when the user switches apps by swiping from the bottom of the screen
window.addEventListener('touchcancel', e => {
if (Capacitor.isNativePlatform()) {
msdewitt marked this conversation as resolved.
Show resolved Hide resolved
Haptics.notification({ type: NotificationType.Warning })
}
this.props.onCancel?.({ clientStart: this.clientStart, e })
this.reset()
})
Expand All @@ -162,6 +167,9 @@ class MultiGesture extends React.Component<MultiGestureProps> {
if (this.props.shouldCancelGesture?.()) {
this.props.onCancel?.({ clientStart: this.clientStart, e })
gestureStore.update('')
if (Capacitor.isNativePlatform()) {
Haptics.notification({ type: NotificationType.Warning })
}
this.abandon = true
return
}
Expand All @@ -179,6 +187,9 @@ class MultiGesture extends React.Component<MultiGestureProps> {
this.scrollYStart = window.scrollY
if (this.props.onStart) {
this.props.onStart({ clientStart: this.clientStart!, e })
if (Capacitor.isNativePlatform()) {
Haptics.impact({ style: ImpactStyle.Light })
}
}
return
}
Expand All @@ -203,6 +214,9 @@ class MultiGesture extends React.Component<MultiGestureProps> {
// append the gesture to the sequence and call the onGesture handler
this.sequence += g
this.props.onGesture?.({ gesture: g, sequence: this.sequence, clientStart: this.clientStart!, e })
if (Capacitor.isNativePlatform()) {
Haptics.impact({ style: ImpactStyle.Light })
}
gestureStore.update(this.sequence)
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/components/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Test:
- Overlay hidden on touch "leave"

*/
import { Haptics } from '@capacitor/haptics'
import React, { FC, useCallback, useEffect, useRef, useState } from 'react'
import { shallowEqual, useDispatch, useSelector } from 'react-redux'
import { css, cva, cx } from '../../styled-system/css'
Expand Down Expand Up @@ -137,6 +138,7 @@ const Toolbar: FC<ToolbarProps> = ({ customize, onSelect, selected }) => {

if (scrollDifference >= 5) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry the difference isn't 15px but 5 pixels right now.

deselectPressingToolbarId()
Haptics.selectionChanged()
}

updateArrows()
Expand Down
2 changes: 1 addition & 1 deletion src/components/ToolbarButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ const ToolbarButton: FC<ToolbarButtonProps> = ({
}),
)}
onMouseLeave={onMouseLeave}
{...fastClick(tapUp, tapDown, undefined, touchMove)}
{...fastClick(tapUp, true, tapDown, undefined, touchMove)}
>
{
// selected top dash
Expand Down
9 changes: 8 additions & 1 deletion src/hooks/useDragHold.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Capacitor } from '@capacitor/core'
import { Haptics } from '@capacitor/haptics'
import { useCallback, useEffect, useState } from 'react'
import { useDispatch } from 'react-redux'
import DragThoughtZone from '../@types/DragThoughtZone'
Expand Down Expand Up @@ -35,6 +37,9 @@ const useDragHold = ({
if (disabled) return
setIsPressed(true)
dispatch([dragHold({ value: true, simplePath, sourceZone })])
if (Capacitor.isNativePlatform()) {
Haptics.selectionStart()
}
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[],
Expand All @@ -51,7 +56,9 @@ const useDragHold = ({

if (state.dragHold) {
dispatch([dragHold({ value: false }), !hasMulticursor(state) ? alert(null) : null])

if (Capacitor.isNativePlatform()) {
Haptics.selectionEnd()
}
if (toggleMulticursorOnLongPress) {
dispatch(toggleMulticursor({ path: simplePath }))
}
Expand Down
2 changes: 2 additions & 0 deletions src/hooks/useLongPress.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Haptics } from '@capacitor/haptics'
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { useDispatch } from 'react-redux'
import { editingActionCreator as editing } from '../actions/editing'
Expand Down Expand Up @@ -41,6 +42,7 @@ const useLongPress = (
clientCoords.current = { x: e.touches?.[0]?.clientX, y: e.touches?.[0]?.clientY }
}
onTouchStart?.()
Haptics.selectionStart()
Copy link
Contributor

Choose a reason for hiding this comment

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

This should go next to onLongPressStart, where the long press actually starts.

start only starts the timer.


// cast Timeout to number for compatibility with clearTimeout
clearTimeout(timerIdRef.current)
Expand Down
5 changes: 5 additions & 0 deletions src/hooks/useToolbarLongPress.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Capacitor } from '@capacitor/core'
import { Haptics } from '@capacitor/haptics'
import { useCallback, useEffect, useMemo, useState } from 'react'
import { useDispatch } from 'react-redux'
import DragShortcutZone from '../@types/DragShortcutZone'
Expand Down Expand Up @@ -29,6 +31,9 @@ const useToolbarLongPress = ({
if (disabled) return
setIsPressed(true)
dispatch(toolbarLongPress({ shortcut, sourceZone }))
if (Capacitor.isNativePlatform()) {
Haptics.selectionStart()
}
}, [disabled, dispatch, shortcut, sourceZone])

/** Turn off isPressed and dismiss an alert when long press ends. */
Expand Down
11 changes: 9 additions & 2 deletions src/util/fastClick.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { Capacitor } from '@capacitor/core'
import { Haptics, ImpactStyle } from '@capacitor/haptics'
import _ from 'lodash'
import { isTouch } from '../browser'

Expand All @@ -13,6 +15,9 @@ const fastClick = isTouch
// triggered on mouseup or touchend
// cancelled if the user scroll or drags
tapUp: (e: React.TouchEvent) => void,

isHaptics: boolean = false,

Comment on lines +18 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

The existing fastClick interface is not very conducive to adding new options unfortunately. Sticking isHaptics in between tapUp and tapDown is not the most intuitive.

I can think of two approaches that would be a bit cleaner:

  1. Convert all of the options to named parameters.
  2. Leave the existing parameters as-is and add isHaptics to a new options object at the end:
..., { isHaptics?: boolean } = {})

// triggered on mousedown or touchstart
tapDown?: (e: React.TouchEvent) => void,
// triggered when tapUp is cancelled due to scrolling or dragging
Expand All @@ -27,7 +32,6 @@ const fastClick = isTouch
const y = e.touches[0].clientY
touchStart = { x, y }
}

msdewitt marked this conversation as resolved.
Show resolved Hide resolved
tapDown?.(e)
},
// cancel tap if touchmove exceeds threshold (e.g. with scrolling or dragging)
Expand All @@ -42,6 +46,9 @@ const fastClick = isTouch
}
}, 16.666),
onTouchEnd: (e: React.TouchEvent) => {
if (Capacitor.isNativePlatform() && isHaptics) {
Haptics.impact({ style: ImpactStyle.Light })
}
let cancel = !touchStart

if (touchStart && e.changedTouches.length > 0) {
Expand All @@ -61,7 +68,7 @@ const fastClick = isTouch
touchStart = null
},
})
: (tapUp: (e: React.MouseEvent) => void, tapDown?: (e: React.MouseEvent) => void) => ({
: (tapUp: (e: React.MouseEvent) => void, isHaptics: boolean = false, tapDown?: (e: React.MouseEvent) => void) => ({
onMouseUp: tapUp,
...(tapDown ? { onMouseDown: tapDown } : null),
})
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1928,6 +1928,15 @@ __metadata:
languageName: node
linkType: hard

"@capacitor/haptics@npm:^6.0.2":
version: 6.0.2
resolution: "@capacitor/haptics@npm:6.0.2"
peerDependencies:
"@capacitor/core": ^6.0.0
checksum: 10c0/c514d3b822541b3ac09625a6dd1647c9a3f94f005470c318da9b680d85024d1938da2aeea76d2641c513126bb836e1fef665bb6a84c7d39a90c6d16237b266d5
languageName: node
linkType: hard

"@capacitor/ios@npm:^6.2.0":
version: 6.2.0
resolution: "@capacitor/ios@npm:6.2.0"
Expand Down Expand Up @@ -7658,6 +7667,7 @@ __metadata:
"@capacitor/cli": "npm:^6.2.0"
"@capacitor/clipboard": "npm:^6.0.2"
"@capacitor/core": "npm:^6.2.0"
"@capacitor/haptics": "npm:^6.0.2"
"@capacitor/ios": "npm:^6.2.0"
"@capacitor/keyboard": "npm:^6.0.2"
"@capacitor/status-bar": "npm:^6.0.1"
Expand Down