Skip to content
This repository has been archived by the owner on Aug 11, 2024. It is now read-only.

"Quote in new note" action #1071

Merged
merged 3 commits into from
Jan 25, 2024
Merged

"Quote in new note" action #1071

merged 3 commits into from
Jan 25, 2024

Conversation

bfollington
Copy link
Collaborator

@bfollington bfollington commented Jan 15, 2024

Fixes #871

  • MVP quoting feature
Screen.Recording.2024-01-15.at.6.13.26.pm.mov

@bfollington bfollington force-pushed the 2024-01-15-quote-note branch from 5a09c52 to 396e7cc Compare January 15, 2024 07:15
@bfollington bfollington force-pushed the 2024-01-15-quote-note branch from 396e7cc to 8d043ae Compare January 17, 2024 05:25
Comment on lines 615 to 622
case let .requestQuoteInNewDetail(address):
return .pushDetail(
.editor(
MemoEditorDetailDescription(
fallback: "\(address.markup)\n\n"
)
)
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This results in an untitled editor, preloaded with the link and the cursor autofocused in a new block.

There is one UX rough edge, if you back out of the new editor this opens, then the app will automatically save the note... which creates a "garbage" note. Perhaps we should have some clever logic that never saves a note unless it contains something other than / in addition to a single slashlink?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is fine?

@bfollington bfollington marked this pull request as ready for review January 17, 2024 05:36
Comment on lines +67 to +68
"Quote in new note",
systemImage: "quote.opening"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I welcome discussion on the best wording and icon for this action.

To me "linking" is inserting a slashlink into an existing note and "quoting" is creating a new note that references / reflects on another note (using linking).

Copy link
Collaborator

Choose a reason for hiding this comment

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

SGTM

@bmann
Copy link

bmann commented Jan 17, 2024

Clipping might be another word. I’m clipping it into my own notebook.

(This looks good!)

Copy link
Collaborator

@gordonbrander gordonbrander left a comment

Choose a reason for hiding this comment

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

LGTM.

My one UX feedback is that I think it might be nicer to place the cursor above the link, with the link on it's own "paragraph" below? I'm imagining what this will look like with transcludes support, and by analogy to quote tweet, you probably want your thoughts above the transcluded content, not below it.

Comment on lines 615 to 622
case let .requestQuoteInNewDetail(address):
return .pushDetail(
.editor(
MemoEditorDetailDescription(
fallback: "\(address.markup)\n\n"
)
)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is fine?

Comment on lines +67 to +68
"Quote in new note",
systemImage: "quote.opening"
Copy link
Collaborator

Choose a reason for hiding this comment

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

SGTM

@bfollington
Copy link
Collaborator Author

LGTM.

My one UX feedback is that I think it might be nicer to place the cursor above the link, with the link on it's own "paragraph" below? I'm imagining what this will look like with transcludes support, and by analogy to quote tweet, you probably want your thoughts above the transcluded content, not below it.

I implemented this and I think it is better, going to run with putting the cursor ahead of the quoted content.

@bfollington bfollington merged commit 1d99b0e into main Jan 25, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Link to this" action
3 participants