[WEB-72] Implement checklists #382
Open
Conversation
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
lauraw0
suggested changes
Sep 4, 2023
There was a problem hiding this comment.
nice nice looks pretty dang good!! just a couple changes needed (as detailed in the comments) plus the following notes:
- Pls change it so that the checkboxes are reflected in the published blogs. This may possibly require some refactoring as you will need to not only keep track of the type (e.g. checklist, quote, code....) but also whether the items are checked/crossed out. This means you may need to add an optional type to the type interfaces.
Editor
Published blog
- Sort of tied to the above point. After you have corrected the interface you will need to ensure that checked items remains checked after refresh. Currently your checked items become unchecked due to the checked value being held in a usestate.
| @@ -0,0 +1,23 @@ | |||
| import React, { MouseEventHandler } from 'react'; | |||
| import { StyledButton, buttonProps, scaleRate } from './small_buttons-Styled'; | |||
| import { ReactComponent as Code } from 'src/cse-ui-kit/assets/checklist-button.svg'; | |||
There was a problem hiding this comment.
Maybe rename the component as Checklist instead
| checked={checked} | ||
| onChange={event => { | ||
| // Figure out why ReactEditor.findPath does not work | ||
| // const path = ReactEditor.findPath(editor as ReactEditor, leaf); |
There was a problem hiding this comment.
Your current implementation works so you can just remove these comments but if you wanted to know why the ReactEditor.findPath() doesn't work, i think these could possibly be the reasons:
- the editor needs to be passed in from the EditorBlock component
- this findPath function is specifically finding a node (https://docs.slatejs.org/libraries/slate-react/react-editor#reacteditor.findpath-editor-reacteditor-node-node-path) but here you are rendering the checklists as leaves. This means that the function will never return the current leaf. You can resolve this by changing the checklists into nodes. NOTE: you can think of a node as sort of a big wrapper/container whilst leaves are sort of the children with decorations. However, I do have some doubts about this cause in that case it should in theory find the leaf level text node surrounding this lead? Not completely sure without delving into it too deeply.
- This is probably the most likely reason: the structure of our editor/leaves/nodes does not follow typical slate operations and therefore any functions that involve traversing through the editor path is doomed to fail
Comment on lines
+57
to
+60
| <div | ||
| style={{ userSelect: "none" }} | ||
| contentEditable={false} | ||
| > |
There was a problem hiding this comment.
This div doesn't appear to do much so you can remove it
| @@ -46,6 +48,51 @@ const Text = styled.span<{ | |||
| background-color: ${(props) => props.code ? "#eee" : "#fff"}; | |||
| `; | |||
|
|
|||
|
|
|||
| const CheckListItemElement = ({children, attributes, leaf}: any) => { | |||
| const readOnly = useReadOnly() | |||
There was a problem hiding this comment.
You can remove the use of this hook as it is unlikely for us to set the editor to readonly. We can always add it back in when needed though :D
Comment on lines
+123
to
+124
| checklist: leaf.checklist ?? false, | ||
| checked: leaf.checklist ?? false, |
There was a problem hiding this comment.
This doesn't appear to be used elsewhere so you can remove it. Let me know if i missed its use case
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why the changes are required?
Users may want to apply a checklist within the editor similar to the numbered and bulleted lists
Changes