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

Third bunch of possible undefined thoughts #2744

Merged
merged 6 commits into from
Jan 3, 2025
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 3 additions & 1 deletion src/actions/bulletColor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export const bulletColorActionCreator =
(payload: Parameters<typeof bulletColor>[1]): Thunk =>
(dispatch, getState) => {
const state = getState()
const thought = pathToThought(state, state.cursor!)
if (!state.cursor) return
const thought = pathToThought(state, state.cursor)
if (!thought) return
const thoughtText = stripTags(thought.value)
const fullySelected =
(selection.text()?.length === 0 && thoughtText.length !== 0) || selection.text()?.length === thoughtText.length
Expand Down
4 changes: 2 additions & 2 deletions src/actions/formatLetterCase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export const formatLetterCaseActionCreator =
const cursor = state.cursor
if (!cursor) return

const thought = pathToThought(state, cursor)
const originalThoughtValue = thought.value
const originalThoughtValue = pathToThought(state, cursor)?.value
if (originalThoughtValue === undefined) return

const updatedThoughtValue = applyLetterCase(command, originalThoughtValue)
const simplePath = simplifyPath(state, cursor)
Expand Down
1 change: 1 addition & 0 deletions src/actions/formatWithTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const formatWithTagActionCreator =
const state = getState()
if (!state.cursor) return
const thought = pathToThought(state, state.cursor)
if (!thought) return
const simplePath = thoughtToPath(state, thought.id)
suppressFocusStore.update(true)

Expand Down
20 changes: 11 additions & 9 deletions src/actions/swapNote.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,17 @@ const swapNote = (state: State) => {
const newRank = getRankAfter(state, appendToPath(simplePath, noteId))
const note = pathToThought(state, oldPath)

return reducerFlow([
moveThought({ oldPath, newPath, newRank }),
// delete =note
deleteThought({
pathParent: cursor,
thoughtId: noteId,
}),
setCursor({ offset: note.value.length, path: newPath }),
])(state)
return note
? reducerFlow([
moveThought({ oldPath, newPath, newRank }),
// delete =note
deleteThought({
pathParent: cursor,
thoughtId: noteId,
}),
setCursor({ offset: note.value.length, path: newPath }),
])(state)
: null
},
]
: // if the cursor thought does not have a note, swap it with its parent's note (or create a note if one does not exist)
Expand Down
5 changes: 3 additions & 2 deletions src/components/DropHover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,15 @@ const DropHoverIfVisible = ({
// const distance = state.cursor ? Math.max(0, Math.min(MAX_DISTANCE_FROM_CURSOR, state.cursor.length - depth!)) : 0

const value = getThoughtById(state, head(simplePath))?.value

const prevChild = prevChildId && getThoughtById(state, prevChildId)
return (
value !== undefined &&
(isThoughtHovering || isSubthoughtsHovering) &&
draggingThoughtValue &&
// check if it's alphabetically previous to current thought
compareReasonable(draggingThoughtValue, value) <= 0 &&
// check if it's alphabetically next to previous thought if it exists
(!prevChildId || compareReasonable(draggingThoughtValue, getThoughtById(state, prevChildId).value) === 1)
(!prevChild || compareReasonable(draggingThoughtValue, prevChild.value) === 1)
)
})

Expand Down
11 changes: 8 additions & 3 deletions src/hooks/useDragAndDropSubThought.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const canDrop = (props: DroppableSubthoughts, monitor: DropTargetMonitor): boole
const distance = state.cursor ? state.cursor.length - thoughtsTo.length - 1 : 0
const isHidden = distance >= visibleDistanceAboveCursor(state) && !isExpandedTop()
const isDescendant = isDescendantPath(thoughtsTo, thoughtsFrom)
const divider = isDivider(getThoughtById(state, head(thoughtsTo)).value)
const divider = isDivider(getThoughtById(state, head(thoughtsTo))?.value)

const showContexts = thoughtsTo && isContextViewActive(state, thoughtsTo)

Expand Down Expand Up @@ -118,13 +118,18 @@ const drop = (props: DroppableSubthoughts, monitor: DropTargetMonitor) => {
if (equalPath(thoughtsFrom, props.simplePath)) return

// cannot move root or em context or target is divider
if (isDivider(thoughtTo.value) || (isRootOrEM && !sameContext)) {
if (isDivider(thoughtTo?.value) || (isRootOrEM && !sameContext)) {
store.dispatch(
error({ value: `Cannot move the ${isEM(thoughtsFrom) ? 'em' : 'home'} context to another context.` }),
)
return
}

if (!thoughtTo) {
console.warn(`Cannot drop ${thoughtFrom} on itself. Aborting drop.`)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This console.warn is not accurate, as dropping a thought on itself is already detected and short circuited using the equalPath check above. Please update the message or remove the console.warn completely, combining it with the first return statement.


store.dispatch(
moveThought({
oldPath: thoughtsFrom,
Expand All @@ -134,7 +139,7 @@ const drop = (props: DroppableSubthoughts, monitor: DropTargetMonitor) => {
)

// alert user of move to another context
if (!sameContext) {
if (!sameContext && thoughtTo && thoughtFrom) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't necessary due to the short circuit above.

// wait until after MultiGesture has cleared the error so this alert does no get cleared
setTimeout(() => {
const alertFrom = '"' + ellipsize(thoughtFrom.value) + '"'
Expand Down
5 changes: 4 additions & 1 deletion src/initialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,10 @@ const windowEm = {
getLexeme: withState(getLexeme),
getLexemeContexts: withState((state: State, value: string) => {
const contexts = getLexeme(state, value)?.contexts || []
return contexts.map(id => thoughtToContext(state, getThoughtById(state, id)?.parentId))
return contexts
.map(id => getThoughtById(state, id))
.filter(Boolean)
.map(thought => thoughtToContext(state, thought.parentId))
}),
getAllChildrenByContext: withState((state: State, context: Context) =>
getAllChildren(state, contextToThoughtId(state, context) || null),
Expand Down
2 changes: 1 addition & 1 deletion src/redux-middleware/__tests__/pullQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ it('do not repopulate deleted thought', async () => {
})

const parentEntryChild = contextToThought(store.getState(), [''])
expect(parentEntryChild).toBe(null)
expect(parentEntryChild).toBe(undefined)
})

it('load buffered thoughts', async () => {
Expand Down
8 changes: 4 additions & 4 deletions src/selectors/nextThought.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ import parentOf from '../util/parentOf'
const nextContext = (state: State, path: Path) => {
// use rootedParentOf(path) instead of thought.parentId since we need to cross the context view
const parent = getThoughtById(state, head(rootedParentOf(state, path)))
const contexts = getContextsSortedAndRanked(state, parent.value)
const contexts = parent ? getContextsSortedAndRanked(state, parent.value) : []
// find the thought in the context view
const index = contexts.findIndex(cx => getThoughtById(state, cx.id).parentId === head(path))
const index = contexts.findIndex(cx => getThoughtById(state, cx.id)?.parentId === head(path))
// get the next context
const nextContextId = contexts[index + 1]?.id
const nextContext = nextContextId ? getThoughtById(state, nextContextId) : null
Expand All @@ -33,11 +33,11 @@ const nextContext = (state: State, path: Path) => {
/** Gets the first context in a context view. */
const firstContext = (state: State, path: Path): Path | null => {
const thought = getThoughtById(state, head(path))
const contexts = getContextsSortedAndRanked(state, thought.value)
const contexts = thought ? getContextsSortedAndRanked(state, thought.value) : []

// if context view is empty, move to the next thought
const firstContext = getThoughtById(state, contexts[0]?.id)
return contexts.length > 1
return firstContext && contexts.length > 1
? appendToPath(path, firstContext.parentId)
: nextThought(state, path, { ignoreChildren: true }) // eslint-disable-line @typescript-eslint/no-use-before-define
}
Expand Down
2 changes: 1 addition & 1 deletion src/selectors/parentOfThought.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const parentOfThought = (state: State, thoughtId: ThoughtId): Thought | null =>
const thought = getThoughtById(state, thoughtId)
if (!thought) return null
const parentThought = getThoughtById(state, thought.parentId)
return parentThought
return parentThought ?? null
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

preserve fn signature

}

export default parentOfThought
4 changes: 2 additions & 2 deletions src/selectors/prevContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import rootedParentOf from './rootedParentOf'
const prevContext = (state: State, path: Path) => {
// use rootedParentOf(path) instead of thought.parentId since we need to cross the context view
const parent = getThoughtById(state, head(rootedParentOf(state, path)))
const contexts = getContextsSortedAndRanked(state, parent.value)
const contexts = parent ? getContextsSortedAndRanked(state, parent.value) : []
// find the thought in the context view
const index = contexts.findIndex(cx => getThoughtById(state, cx.id).parentId === head(path))
const index = contexts.findIndex(cx => getThoughtById(state, cx.id)?.parentId === head(path))
const context = contexts[index - 1]
return context ? getThoughtById(state, context.parentId) : null
}
Expand Down
8 changes: 6 additions & 2 deletions src/selectors/prevSibling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ export const prevSibling = (
if (!thought || (!state.showHiddenThoughts && isAttribute(thought.value))) return null

const parentPath = rootedParentOf(state, path)
const parent = getThoughtById(state, head(parentPath))
const showContexts = showContextsForced ?? isContextViewActive(state, parentPath)

// siblings, including the current thought
const siblings = showContexts
? getContextsSortedAndRanked(state, getThoughtById(state, head(parentPath)).value)
? parent
? getContextsSortedAndRanked(state, parent.value)
: []
: getChildrenSorted(state, thought.parentId)

// in context view, we need to match the context's parentId, since all context's ids refer to lexeme instances
Expand All @@ -45,9 +48,10 @@ export const prevSibling = (
}

const prev = siblings[index - 1]
if (!prev) return null

// in context view, we select then parent since prev again refers to the lexeme instance
return prev ? (showContexts ? getThoughtById(state, prev.parentId) : prev) : null
return showContexts ? (getThoughtById(state, prev.parentId) ?? null) : prev
}

export default prevSibling
2 changes: 1 addition & 1 deletion src/stores/commandStateStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const updateCommandState = () => {
const action =
selection.isActive() && selection.isOnThought()
? getCommandState(selection.html() ?? '')
: getCommandState(pathToThought(state, state.cursor).value)
: getCommandState(pathToThought(state, state.cursor)?.value ?? '')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

on that end we have a path.length === 0 default case, so fallback is fine here

commandStateStore.update(action)
}

Expand Down
4 changes: 2 additions & 2 deletions src/test-helpers/contextToThought.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import getThoughtById from '../selectors/getThoughtById'
/**
* Converts a Context to a Thought. If more than one thought has the same value in the same context, traveerses the first.
*/
const contextToThought = (state: State, context: Context): Thought | null => {
const contextToThought = (state: State, context: Context): Thought | undefined => {
const id = contextToThoughtId(state, context)
return id ? getThoughtById(state, id) : null
return id ? getThoughtById(state, id) : undefined
}

export default contextToThought
4 changes: 4 additions & 0 deletions src/test-helpers/deleteThoughtAtFirstMatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ const getThoughtAndParentPath = (state: State, at: string[]): [Thought, Path] =>

const thought = pathToThought(state, path)

if (!thought) throw new Error(`Thought not found for path: ${path}`)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's a test file, and we throw an error for empty path above, so thought why not


const pathParent = rootedParentOf(state, path)

return [thought, pathParent]
Expand All @@ -28,6 +30,7 @@ const getThoughtAndParentPath = (state: State, at: string[]): [Thought, Path] =>
*/
const deleteThoughtAtFirstMatch = _.curryRight((state: State, at: string[]) => {
const [thought, pathParent] = getThoughtAndParentPath(state, at)

return deleteThought(state, {
pathParent,
thoughtId: thought.id,
Expand All @@ -41,6 +44,7 @@ export const deleteThoughtAtFirstMatchActionCreator =
(at: Context): Thunk =>
(dispatch, getState) => {
const [thought, pathParent] = getThoughtAndParentPath(getState(), at)

dispatch(
deleteThoughtActionCreator({
pathParent,
Expand Down
3 changes: 2 additions & 1 deletion src/util/getContextMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ const getContextMap = (state: State, lexemes: (Lexeme | undefined)[]) =>
...acc,
...lexeme.contexts.reduce<Index<Context>>((accInner, thoughtId) => {
const thought = getThoughtById(state, thoughtId)
if (!thought) return accInner
return {
...accInner,
[thought.parentId]: unroot(thoughtToContext(state, thought.parentId)!),
[thought.parentId]: unroot(thoughtToContext(state, thought.parentId)),
}
}, {}),
}),
Expand Down
7 changes: 4 additions & 3 deletions src/util/importJSON.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const insertThought = (
},
) => {
const thoughtOld = getThoughtById(state, id)
if (!thoughtOld) return null
const childLastUpdated = block.children[0]?.lastUpdated
const childCreated = block.children[0]?.created
const lastUpdatedInherited =
Expand Down Expand Up @@ -233,10 +234,10 @@ const importJSON = (
{ lastUpdated = timestamp(), updatedBy = clientId, skipRoot = false }: ImportJSONOptions = {},
) => {
const destThought = pathToThought(state, simplePath)
const destEmpty = destThought.value === '' && !anyChild(state, head(simplePath))
const destEmpty = destThought?.value === '' && !anyChild(state, head(simplePath))
// use getNextRank instead of getRankAfter because if dest is not empty then we need to import thoughts inside it
const rankStart = destEmpty ? destThought.rank : getNextRank(state, head(simplePath))
const rankIncrement = getRankIncrement(state, blocks, destThought, rankStart)
const rankStart = destEmpty ? destThought?.rank : getNextRank(state, head(simplePath))
const rankIncrement = destEmpty ? getRankIncrement(state, blocks, destThought, rankStart) : 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

1 is a fallback in getRankIncrement too, so that's my judgment

const pathParent = rootedParentOf(state, simplePath)
const parentId = head(pathParent)

Expand Down
2 changes: 1 addition & 1 deletion src/util/isDivider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** Returns true if the value starts with multiple dashes and should be interpreted as a divider. */
const isDivider = (s: string | null) => s !== null && (s.startsWith('---') || s.startsWith('—-'))
const isDivider = (s: string | null | undefined) => s?.startsWith('---') || s?.startsWith('—-')

export default isDivider
4 changes: 2 additions & 2 deletions src/util/moveLexemeThought.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ const moveLexemeThought = (state: State, lexeme: Lexeme, oldRank: number, newRan
const thought = getThoughtById(state, child)
return (
// remove old context
(thought.rank !== oldRank || child !== id) &&
(thought?.rank !== oldRank || child !== id) &&
// remove new context with duplicate rank
(thought.rank !== newRank || child !== id)
(thought?.rank !== newRank || child !== id)
)
}),
// add new context
Expand Down
2 changes: 1 addition & 1 deletion src/util/noteValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const noteValue = (state: State, id: ThoughtId) => {
const noteId = findDescendant(state, id, '=note')
if (!noteId) return null
const noteThought = getThoughtById(state, noteId)
if (noteThought.pending) return null
if (noteThought?.pending) return null
return firstVisibleChild(state, noteId!)?.value ?? null
}

Expand Down
2 changes: 1 addition & 1 deletion src/util/removeDuplicatedContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const removeDuplicatedContext = (state: State, lexeme: Lexeme, context: Context)
...lexeme,
contexts: (lexeme.contexts || []).filter(child => {
const thought = getThoughtById(state, child)
return thought.parentId !== contextToThoughtId(state, context)
return thought?.parentId !== contextToThoughtId(state, context)
}),
}
}
Expand Down
Loading