-
Notifications
You must be signed in to change notification settings - Fork 124
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
Changes from 5 commits
b4decc7
9fa99ad
ce82e0a
01b8c52
d77905b
5c816ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
|
||
|
@@ -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 | ||
} | ||
|
||
store.dispatch( | ||
moveThought({ | ||
oldPath: thoughtsFrom, | ||
|
@@ -134,7 +139,7 @@ const drop = (props: DroppableSubthoughts, monitor: DropTargetMonitor) => { | |
) | ||
|
||
// alert user of move to another context | ||
if (!sameContext) { | ||
if (!sameContext && thoughtTo && thoughtFrom) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) + '"' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. preserve fn signature |
||
} | ||
|
||
export default parentOfThought |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 ?? '') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on that end we have a |
||
commandStateStore.update(action) | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}`) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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] | ||
|
@@ -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, | ||
|
@@ -41,6 +44,7 @@ export const deleteThoughtAtFirstMatchActionCreator = | |
(at: Context): Thunk => | ||
(dispatch, getState) => { | ||
const [thought, pathParent] = getThoughtAndParentPath(getState(), at) | ||
|
||
dispatch( | ||
deleteThoughtActionCreator({ | ||
pathParent, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
||
|
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 |
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
console.warn
is not accurate, as dropping a thought on itself is already detected and short circuited using theequalPath
check above. Please update the message or remove theconsole.warn
completely, combining it with the first return statement.