Skip to content

Commit

Permalink
Merge pull request #786 from pushkov-fedor/disallow-duplicate-thoughts
Browse files Browse the repository at this point in the history
  • Loading branch information
raineorshine authored Sep 2, 2020
2 parents b6f81c1 + 4484bf8 commit 9c2bf2c
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 29 deletions.
23 changes: 20 additions & 3 deletions src/action-creators/alert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,36 @@ import { ActionCreator } from '../types'
interface Options {
alertType?: string,
showCloseLink?: boolean,
clearTimeout?: number,
}

let clearAlertTimeoutId: number | null = null// eslint-disable-line fp/no-let

/**
* Dispatches an alert action.
*
* @param value The string or React Component that will be rendered in the alert.
* @param showCloseLink Show a small 'x' in the upper right corner that allows the user to close the alert. Default: true.
* @param type An arbitrary alert type that can be added to the alert. This is useful if specific alerts needs to be detected later on, for example, to determine if the alert should be closed, or if it has been superceded by a different alert type.
* @param clearTimeout Timeout after which alert will be cleared.
*/
const alert = (value: string | FunctionComponent | null, { alertType, showCloseLink }: Options = {}): ActionCreator => (dispatch, getState) => {

const alert = (value: string | FunctionComponent | null, { alertType, showCloseLink, clearTimeout }: Options = {}): ActionCreator => (dispatch, getState) => {
const { alert } = getState()

if (clearTimeout) {
// if clearAlertTimeoutId !== null, it means that previous alert hasn't been cleared yet. In this case cancel previous timeout and start new.
clearAlertTimeoutId && window.clearTimeout(clearAlertTimeoutId)
clearAlertTimeoutId = window.setTimeout(() => {
dispatch({
type: 'alert',
alertType,
showCloseLink,
value: null,
})
clearAlertTimeoutId = null
}, clearTimeout)
}

if (alert && alert.value === value) return

dispatch({
Expand All @@ -24,7 +42,6 @@ const alert = (value: string | FunctionComponent | null, { alertType, showCloseL
showCloseLink,
value,
})

}

export default alert
48 changes: 35 additions & 13 deletions src/action-creators/newThought.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { ActionCreator } from '../types'
import { ActionCreator, Context } from '../types'
import { isMobile, isSafari } from '../browser'
import { TUTORIAL_STEP_START } from '../constants'
import { getSetting, hasChild, isContextViewActive } from '../selectors'
import { ROOT_TOKEN, TUTORIAL_STEP_START } from '../constants'
import { getSetting, getThoughts, hasChild, isContextViewActive } from '../selectors'
import { alert } from '../action-creators'

// util
import {
Expand All @@ -11,6 +12,20 @@ import {
headValue,
pathToContext,
} from '../util'
import { State } from '../util/initialState'

interface Alert {
type: 'alert',
value: string | null,
alertType: string,
}

/** Split editingValue by offset and check if splitted parts are duplicate with siblings. */
const isDuplicateOnSplit = (offset: number, context: Context | null, state: State) => {
const { editingValue } = state
const siblings = context && getThoughts(state, context)
return siblings && editingValue && siblings.some(sibling => sibling.value === editingValue.substring(0, offset) || sibling.value === editingValue.substring(offset))
}

/**
* Creates a new thought.
Expand All @@ -20,8 +35,7 @@ import {
*/
const newThought = ({ offset, preventSplit, value = '' }: { offset: number, preventSplit?: boolean, value: string }): ActionCreator => (dispatch, getState) => {
const state = getState()
const { cursor } = state

const { cursor, editingValue } = state
const tutorial = getSetting(state, 'Tutorial') !== 'Off'
const tutorialStep = +!getSetting(state, 'Tutorial Step')

Expand All @@ -37,21 +51,29 @@ const newThought = ({ offset, preventSplit, value = '' }: { offset: number, prev

const showContexts = cursor && isContextViewActive(state, contextOf(pathToContext(cursor)))

const context = cursor && (showContexts && cursor.length > 2 ? pathToContext(contextOf(contextOf(cursor)))
: !showContexts && cursor.length > 1 ? pathToContext(contextOf(cursor))
: [ROOT_TOKEN])
// split the thought at the selection
// do not split at the beginning of a line as the common case is to want to create a new thought after, and shift + Enter is so near
// do not split with gesture, as Enter is avialable and separate in the context of mobile
const split = !preventSplit && cursor && isFocusOnEditable && !showContexts && !value && offset > 0 && offset < headValue(cursor).length

const split = !preventSplit && cursor && isFocusOnEditable && !showContexts && !value && offset > 0 && editingValue && offset < editingValue.length
if ((!split || !uneditable) && isMobile && isSafari) {
asyncFocus()
}

dispatch(split
? uneditable && cursor
if (split) {
if (isDuplicateOnSplit(offset, context, state)) {
dispatch(alert('Duplicate thoughts are not allowed within the same context.', { alertType: 'duplicateThoughts', clearTimeout: 2000 }))
return
}
dispatch(uneditable && cursor
? { type: 'error', value: `"${ellipsize(headValue(cursor))}" is uneditable and cannot be split.` }
: { type: 'splitThought', offset }
: { type: 'newThought', value }
)
: { type: 'splitThought', offset })
return
}

dispatch({ type: 'newThought', value })

}

export default newThought
73 changes: 68 additions & 5 deletions src/components/Editable.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useRef } from 'react'
import React, { Dispatch, useEffect, useRef } from 'react'
import { connect } from 'react-redux'
import { throttle } from 'lodash'
import he from 'he'
Expand Down Expand Up @@ -114,6 +114,36 @@ interface EditableProps {
onKeyDownAction?: () => void,
}

interface Alert {
type: 'alert',
value: string | null,
alertType: string,
}

/** Toggle duplication alert. Use closure for storing timeoutId in order to cancel dispatching alert if it's necessary. */
const duplicateAlertToggler = () => {
let timeoutId: number | undefined // eslint-disable-line fp/no-let
return (show: boolean, dispatch: Dispatch<Alert>) => {
const { alert } = store.getState()
if (show) {
timeoutId = window.setTimeout(() => {
dispatch({ type: 'alert', value: 'Duplicate thoughts are not allowed within the same context.', alertType: 'duplicateThoughts' })
timeoutId = undefined
}, 2000)
return
}
if (timeoutId) {
window.clearTimeout(timeoutId)
timeoutId = undefined
}
if (alert && alert.alertType === 'duplicateThoughts') {
setTimeout(() => dispatch({ type: 'alert', value: null, alertType: 'duplicateThoughts' }))
}
}
}

const showDuplicationAlert = duplicateAlertToggler()

/**
* An editable thought with throttled editing.
* Use rank instead of headRank(thoughtsRanked) as it will be different for context view.
Expand Down Expand Up @@ -142,6 +172,9 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff

// store ContentEditable ref to update DOM without re-rendering the Editable during editing
const contentRef = React.useRef<HTMLInputElement>(null)
if (contentRef.current) {
contentRef.current.style.opacity = '1.0'
}

// =style attribute on the thought itself
const styleAttr = getStyle(state, thoughtsRanked)
Expand All @@ -168,7 +201,6 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff

/** Set the cursor on the thought. */
const setCursorOnThought = ({ editing }: { editing?: boolean } = {}) => {

const { cursorBeforeEdit, cursor } = store.getState() // use fresh state

const isEditing = equalPath(cursorBeforeEdit, thoughtsResolved)
Expand Down Expand Up @@ -208,7 +240,6 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff
}

const thought = getThought(state, oldValue)

if (thought) {
dispatch({ type: 'existingThoughtChange', context, showContexts, oldValue, newValue, rankInContext: rank, thoughtsRanked, contextChain })

Expand Down Expand Up @@ -289,6 +320,7 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff
return () => {
throttledChangeRef.current.flush()
shortcutEmitter.off('shortcut', flush)
showDuplicationAlert(false, dispatch)
}
}, [isEditing, cursorOffset])

Expand All @@ -308,8 +340,11 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff
// disabled={readonly} removes contenteditable property

dispatch(setEditingValue(newValue))

if (newValue === oldValue) {
if (contentRef.current) {
contentRef.current.style.opacity = '1.0'
}
showDuplicationAlert(false, dispatch)

if (readonly || uneditable || options) invalidStateError(null)

Expand All @@ -321,6 +356,24 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff
}

const oldValueClean = oldValue === EM_TOKEN ? 'em' : ellipsize(oldValue)

const thoughtsInContext = getThoughts(state, context)
const hasDuplicate = thoughtsInContext.some(thought => thought.value === newValue)
if (hasDuplicate) {
showDuplicationAlert(true, dispatch)
throttledChangeRef.current.cancel() // see above
if (contentRef.current) {
contentRef.current.style.opacity = '0.5'
}
return
}
else {
if (contentRef.current) {
contentRef.current.style.opacity = '1.0'
}
showDuplicationAlert(false, dispatch)
}

if (readonly) {
dispatch({ type: 'error', value: `"${ellipsize(oldValueClean)}" is read-only and cannot be edited.` })
throttledChangeRef.current.cancel() // see above
Expand Down Expand Up @@ -394,6 +447,14 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff

const { invalidState } = state
throttledChangeRef.current.flush()
// set editingValue in order to position superscript correctly if edited thought is duplicate
oldValueRef.current && dispatch(setEditingValue(oldValueRef.current))
// reset rendered value to previous non-duplicate
if (contentRef.current) {
contentRef.current.innerHTML = oldValueRef.current
contentRef.current.style.opacity = '1.0'
showDuplicationAlert(false, dispatch)
}

// on blur remove error, remove invalid-option class, and reset editable html
if (invalidState) {
Expand Down Expand Up @@ -424,7 +485,6 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff

// must get new state
const state = store.getState()

// not sure if this can happen, but I observed some glitchy behavior with the cursor moving when a drag and drop is completed so check dragInProgress to be. safe
if (!state.dragInProgress) {

Expand All @@ -447,6 +507,9 @@ const Editable = ({ disabled, isEditing, thoughtsRanked, contextChain, cursorOff
}
clearSelection()
}
else {
dispatch(setEditingValue(value))
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/components/ThoughtAnnotation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
contextOf,
ellipsizeUrl,
equalPath,
// getOffsetWithinContent,
head,
headValue,
pathToContext,
Expand Down Expand Up @@ -45,9 +46,10 @@ interface ThoughtAnnotationProps {
/** Sets the innerHTML of the subthought text. */
const getSubThoughtTextMarkup = (state: State, isEditing: boolean, subthought: { text: string }, thoughts: Context) => {
const labelChildren = getThoughts(state, [...thoughts, '=label'])
const { editingValue } = state
return {
__html: isEditing
? subthought.text
? editingValue && subthought.text !== editingValue ? editingValue : subthought.text
: labelChildren.length > 0
? labelChildren[0].value
: ellipsizeUrl(subthought.text)
Expand Down Expand Up @@ -122,7 +124,6 @@ const ThoughtAnnotation = ({ thoughtsRanked, showContexts, showContextBreadcrumb
>
<UrlIcon />
</a>

return <div className='thought-annotation' style={homeContext ? { height: '1em', marginLeft: 8 } : {}}>

{showContextBreadcrumbs ? <ContextBreadcrumbs thoughtsRanked={contextOf(contextOf(thoughtsRanked))} showContexts={showContexts} /> : null}
Expand All @@ -132,7 +133,6 @@ const ThoughtAnnotation = ({ thoughtsRanked, showContexts, showContextBreadcrumb
: subthoughts.map((subthought, i) => {

const numContexts = subthought.contexts.filter(isNotArchive).length + (isRealTimeContextUpdate ? 1 : 0)

return <React.Fragment key={i}>
{i > 0 ? ' ' : null}
<div className={classNames({
Expand Down
37 changes: 35 additions & 2 deletions src/shortcuts/deleteEmptyThoughtOrOutdent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ import { contextOf, ellipsize, headValue, isDivider, isDocumentEditable, pathToC
import {
getChildren,
getThoughtBefore,
getThoughts,
getThoughtsRanked,
hasChild,
isContextViewActive,
lastThoughtsFromContextChain,
splitChain,
} from '../selectors'
import { State } from '../util/initialState'
import { RANKED_ROOT } from '../constants'
import { alert } from '../action-creators'

interface Error {
type: 'error',
Expand All @@ -25,6 +28,12 @@ interface Outdent {
type: 'outdent',
}

interface Alert {
type: 'alert',
value: string | null,
alertType: string,
}

/** Returns true if the cursor is on an empty though or divider that can be deleted. */
const canExecuteDeleteEmptyThought = (state: State) => {
const { cursor } = state
Expand Down Expand Up @@ -86,18 +95,42 @@ const canExecuteOutdent = (state: State) => {
getChildren(state, contextOf(pathToContext(cursor))).length === 1
}

/** A selector that returns true if merged thought value is duplicate. */
const isMergedThoughtDuplicate = (state: State) => {
const { cursor, editingValue } = state
if (!cursor) return false
// If we are going to delete empty thought
if (headValue(cursor) === '' || editingValue === '') return false

const prevThought = getThoughtBefore(state, cursor)
if (!prevThought) return false
const contextChain = splitChain(state, cursor)
const showContexts = isContextViewActive(state, pathToContext(contextOf(cursor)))
const thoughtsRanked = lastThoughtsFromContextChain(state, contextChain)
const mergedThoughtValue = prevThought.value + headValue(cursor)
const context = pathToContext(showContexts && contextChain.length > 1 ? contextChain[contextChain.length - 2]
: !showContexts && thoughtsRanked.length > 1 ? contextOf(thoughtsRanked) :
RANKED_ROOT)
const siblings = getThoughts(state, context)
const isDuplicate = !siblings.every(thought => thought.value !== mergedThoughtValue)
return isDuplicate
}

/** A selector that returns true if either the cursor is on an empty thought that can be deleted, or is on an only child that can be outdented. */
const canExecute = (getState: () => State) => {
const state = getState()
return canExecuteOutdent(state) || canExecuteDeleteEmptyThought(state)
}

// eslint-disable-next-line jsdoc/require-jsdoc
const exec = (dispatch: Dispatch<Outdent | ActionCreator>, getState: () => State) => {
const exec = (dispatch: Dispatch<Outdent | Alert | ActionCreator>, getState: () => State) => {
if (canExecuteOutdent(getState())) {
dispatch({ type: 'outdent' })
}
// since canExecute has already evaluated to true, we know that canExecuteDeleteEmptyThought is true
// additional check for duplicates
else if (isMergedThoughtDuplicate(getState())) {
dispatch(alert('Duplicate thoughts are not allowed within the same context.', { alertType: 'duplicateThoughts', clearTimeout: 2000 }))
}
else {
dispatch(deleteEmptyThought)
}
Expand Down
Loading

0 comments on commit 9c2bf2c

Please sign in to comment.