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

[WEB-8] [EDITOR] Basic Delete Block Functionality #341

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

josh-ramos-22
Copy link
Contributor

@josh-ramos-22 josh-ramos-22 commented Mar 14, 2023

Why are the changes needed?

Currently there is no way for a user to delete a content or heading block from the editor. Adding this functionality enables users to rollback on mistakes and format more easily.

Changes

frontend/src/packages/editor/index.tsx

  • Added new styled components BlockWrapper and BlockContainer in order to properly format the delete button with its corresponding block.
  • implemented deleteButtonClickHandler, which deletes the block that its caller is associated with.
  • Stubbed deletionOperation for future OT integration.

frontend/src/packages/editor/types.ts

  • added a key attribute to CustomElement, CustomText, and CMSBlockProps. This key is only used to allow a block to know when to re-render its value when a block above it is deleted.

In frontend/src/cse-ui-kit/

  • added a new component DeleteBlock, the button used to delete a corresponding block.

Screenshots

When 'X' button is hovered over
Screenshot 2023-03-14 204100
Screenshot 2023-03-14 204046

When X button is not hovered over

Screenshot 2023-03-14 204258
Screenshot 2023-03-14 204249

Comments

While this implementation is currently working, there are still things to be done to refine and improve:

  • OT Integration - research and check documentation
  • Story for DeleteBlock component - DeleteBlock.stories.tsx is currently just a stub file. Need to research what a story is used for and how to implement one -> DONE (16/03/23 6:42pm)
  • Proper key generation - Currently the key for each block is just generated with Math.random(). While this produces unique enough keys for our needs, it would be good to use a more robust method such as uuid (currently uuid is failing to import, still investigating why).

@josh-ramos-22 josh-ramos-22 marked this pull request as ready for review March 24, 2023 05:20
Copy link
Contributor

@lauraw0 lauraw0 left a comment

Choose a reason for hiding this comment

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

just a small change needed but besides that it looks good! [EDIT: don't merge quite yet there appears to be an error when accessing the published blog]

Copy link
Contributor

Choose a reason for hiding this comment

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

you can move this entire component into frontend/src/cse-ui-kit/DeleteBlock_button/index.ts That way there isn't a need for an empty file with a singular import :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently this doesn't work since index.ts is a ts file instead of a tsx file :/

All of the existing Button components in cse-ui-kit (eg CreateContentBlock, CreateHeadingBlock) have an similar index.ts file with a single import.

Should I change the file to index.tsx? Or keep it consistent with the other button components?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants