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.

Sign up for GitHub

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

Jump to bottom

isDeletable is inadequate - or just plain wrong. #644

Open
eschiebel opened this issue May 22, 2024 · 0 comments
Open

isDeletable is inadequate - or just plain wrong. #644

eschiebel opened this issue May 22, 2024 · 0 comments

Comments

@eschiebel
Copy link

Describe the bug
The docs imply that craft.rules.canMoveOut can be used as a test for being deletable, but it is only called on dragging.
The isDeletable function will return true as long as it's not the root node or a top-level node, which is probably inconsistent with the canMoveOut rule.

To Reproduce
Steps to reproduce the behavior:

  1. go to the landing page demo at https://craft.js.org/
  2. try to drag the sole button out of the Custom3 "I must have 1 button" section, you cannot
  3. now click the trashcan in the toolbar - the button is deleted

Expected behavior
If you can't drag a component out, you should not be able to delete it.

Screenshots

Additional context
Since drag and drop is problematic for keyboard only users, this is an accessibility issue as well.

Your environment

Software Version(s)
craft.js 0.2.6
React
TypeScript
Browser
npm/Yarn
Operating System
@eschiebel eschiebel changed the title isDeletable is inadequat isDeletable is inadequate - or just plain wrong. May 22, 2024
instructure-gerrit pushed a commit to instructure/canvas-lms that referenced this issue Jul 2, 2024
@eschiebel
Even more final tweaks to the block editor
e51fac0 
- you can now set the background color of the ResourcesSection
- there is now a mechanism for a block to determine if it is deletable
  (because the built-in isDeletable is wrong.
   see prevwong/craft.js#644)

closes nothing yet
flag=block_editor

test plan:
  - create a page with the Resources and Highlights section
  - select it
  > expect the background color button in the toolbar
  - click it and change the color
  > expect the new color
  - delete 2 of the 3 cards
  > expect the 3rd card not to have the trash can in its toolbar

Change-Id: I20d4a0e5502b17060cae3781895ccb218ba8fc2c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/351768
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Jacob DeWar <[email protected]>
QA-Review: Jacob DeWar <[email protected]>
Product-Review: Ed Schiebel <[email protected]>
instructure-gerrit pushed a commit to instructure/canvas-lms that referenced this issue Jul 2, 2024
@eschiebel @SaltNPepa
Even more final tweaks to the block editor
da91949 
- you can now set the background color of the ResourcesSection
- there is now a mechanism for a block to determine if it is deletable
  (because the built-in isDeletable is wrong.
   see prevwong/craft.js#644)

closes nothing yet
flag=block_editor

test plan:
  - create a page with the Resources and Highlights section
  - select it
  > expect the background color button in the toolbar
  - click it and change the color
  > expect the new color
  - delete 2 of the 3 cards
  > expect the 3rd card not to have the trash can in its toolbar

Change-Id: I20d4a0e5502b17060cae3781895ccb218ba8fc2c
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/351768
Tested-by: Service Cloud Jenkins <[email protected]>
Reviewed-by: Jacob DeWar <[email protected]>
QA-Review: Jacob DeWar <[email protected]>
Product-Review: Ed Schiebel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@eschiebel