-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add quote↔text block switching; add block switcher validation #465
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1aed21a
Move heading block `transforms` to before `edit` and `save`
nylen 98a87ea
Disable switching a multi-paragraph text block to a heading block
nylen 0b44dcd
Add transformations between quote and text blocks
nylen decdfcb
Fix lint errors
nylen c30cd92
Add a test for the new functionality
nylen d79463b
Rename quote block 'value' to 'content' (this matches the text block)
nylen 854f1bb
Fix 'from' transformations
nylen a411591
When switching quote to text, include the citation as a paragraph
nylen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Returning
null
above is also an error (no transformation found), we should be consistent.In the same time, I'm wondering if we really need to throw specific errors. I don't see any use case where we'd want to perform different actions depending on the error. Should we show this error somewhere in the UI? I don't think so, it's not relevant for the user I think.
Note that the
transforms
API is not used for block switching only. If we introduce new returns values like these, we'd want to handle them in all "clients" see #457Also, I think we should perform the transformation even if we have data loss (we should limit the data loss) because the user explicitly asks for the transformation, so it's not a surprise for him to get its content changed a bit. For example when switching from a text block to a heading block, I'm not agains concatening with spaces the different paragraphs. If we reject the transformation, this means backspacing from a text block to a heading won't work either.
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.
What would you suggest returning instead of
null
? To me this makes sense because it is a generic failure case which doesn't have specific meaning to the user (something went wrong in the code).I'll address the rest of your comments below.