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

Delete menus in nav menus screen #21379

Closed
wants to merge 4 commits into from

Conversation

draganescu
Copy link
Contributor

@draganescu draganescu commented Apr 3, 2020

Description

Adds delete button to #21281

How has this been tested?

Tested locally by:

  1. Enable the menu edit experiment
  2. Make sure to use a theme that supports menus
  3. Create some menus
  4. Go to Gutenberg -> Navigation (beta)
  5. Start deleting menus

Screenshots

delete-mystery

Types of changes

Possibly breaking other things b/c this required edits in the getMergedItemIds function which is handling some parts of entity edits. Except for this:

  • updates the menu editor component with delete utilities
  • adds a delete action to entities in core/data

Know issues

  • We need a confirmation
  • We need an empty state

@github-actions
Copy link

github-actions bot commented Apr 3, 2020

Size Change: +322 B (0%)

Total Size: 889 kB

Filename Size Change
build/core-data/index.js 10.9 kB +194 B (1%)
build/edit-navigation/index.js 2.77 kB +62 B (2%)
build/edit-navigation/style-rtl.css 296 B +57 B (19%) ⚠️
build/edit-navigation/style.css 297 B +56 B (18%) ⚠️
build/editor/editor-styles-rtl.css 400 B -23 B (5%)
build/editor/editor-styles.css 402 B -24 B (5%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/editor-rtl.css 7.22 kB 0 B
build/block-library/editor.css 7.22 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.53 kB 0 B
build/block-library/style.css 7.54 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.18 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.61 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@draganescu draganescu force-pushed the add/delete-menus-nav-menus-php branch from 3ecd27c to 1329345 Compare April 6, 2020 09:08
@draganescu draganescu marked this pull request as ready for review April 7, 2020 16:33
@draganescu
Copy link
Contributor Author

Given that the work here affects all the parts of Gutenberg that use entities, I will redo this implementation to take into consideration the following:

  • all entity actions need an undo
  • therefore click on delete the entity is marked as deleted in the state
  • but there is an undo level that allows to add the entity back on undo
  • when the entity is marked for deletion we need to keep track of the removed item so that when we save, we also send DELETE calls to the api aside from the normal POST / PUT calls.

Threfore,

  • we don't need a deleteEntityRecord action at all
  • instead, we use editEntityRecord and mark the entity as deleted
    • we need a way for entities marked as deleted to not be rendered
  • and then alter saveEntityRecord to send the required DELETE calls for items marked to be deleted

How all this is exposed in the UI will differ based on the editor (FSE, Post, Navigation).

@draganescu
Copy link
Contributor Author

I will close this in favor of #21486. Also, I will make a separate PR that will attempt to implement entity delete according to the description above. When that is ready we can bring it to the new screen.

@draganescu draganescu closed this Apr 8, 2020
@aristath aristath deleted the add/delete-menus-nav-menus-php branch November 10, 2020 14:29
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