-
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 delete button #130
Comments
Yeah I agree. I made it like that because it was slightly easier the implement, and didn't know where to put the cursor. Should it go to the end of the previous block, or the start of the next block? What if there is no previous block, or no start block? (That one is easy I guess. :D) |
After looking at these more... it's almost as if these two deletion actions should be swapped.
so... Delete icon is deleting the block. |
If there is no other block, we'll have to create an empty slot and put the caret in it, otherwise you can't interact any longer. |
I feel like this particular interaction is too early to optimize for. You are in a writing context, and the backspace is natural to any such interaction. We should make sure backspace works for deleting blocks. Only when we have tried that, and felt in our bones that it's not enough, we can add a trash button, and yes, the trash button if shown next to the block level controls should remove the entire block, and probably not put the cursor anywhere, since you're in a "layout mode" at that point. |
What should happen when you're in a new empty paragraph block, and you hit delete/backspace on your keyboard? It currently mostly works as I expect, but there's a couple extra characters that seem to exist. Taking a video. |
This is a UI prototype limitation of contenteditable. Something something the |
Good to know. :) |
Just in order to test the UI and API a bit more I added a table block. This seems to be a good case where a delete button would be good to have, as all fields are text fields and backspace means deleting a character. There is no way to delete the block. |
We're quite likely to need a delete control in the form of a button or something clickable for embed blocks too, where you're embedding a video, or tweet, or something like that. |
Though I'm increasingly conceding that this button seems necessary (though I'd lean towards an X icon over a trashcan in any case), I still think we should try without this button for starters, and require people to backspace. That being said, I do sort of expect we'll need this thing at some point regardless for mobile. Probably not for text, as backspace seems the most obvious desktop and mobile here, but for the bigger blocks. |
How do you propose we delete an embedded video or tweet, where there is no tinymce editor? |
You'd still be able to select the block and press backspace, no? I was perhaps unclear in the past comment, I don't mean to say I'm down on the button or that we can't ever add it. For embeds, images, galleries, cover images and all complex block, I do anticipate we'll likely need such a button. But it's not a high priority, I would say, as we have something that works, unless I'm missing something. |
You'd still be able to, but how will the user know that works? Embed blocks
present themselves very differently from other editable blocks.
For a long time now, browsers have used backspace to go back in the
browsing history. Users are comfortable with pressing backspace to delete
content if they see a cursor blinking away on the screen, but if they just
focus the tweet block, they don't know what behaviour backspace will have
when they press it. If focusing the tweet returns them to the form with the
input box, and they delete the url, there's no user experience I'm aware of
that says "pressing backspace in a form's text input will get rid of the
form".
…On Tue, May 9, 2017 at 10:39 AM, Joen Asmussen ***@***.***> wrote:
How do you propose we delete an embedded video or tweet, where there is no
tinymce editor?
You'd still be able to select the block and press backspace, no?
I was perhaps unclear in the past comment, I don't mean to say I'm down on
the button or that we can't ever add it. For embeds, images, galleries,
cover images and all complex block, I *do* anticipate we'll likely need
such a button. But it's not a high priority, I would say, as we have
something that works, unless I'm missing something.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#130 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADKSstiWWNYH_q4V0C-eb-9OKXjN9JV-ks5r4DQ4gaJpZM4MKZWL>
.
|
Solid, solid points. I'm swayed for everything but the text blocks. |
Is this decided? Could we have a design? |
I'm still slightly unsure. I mean — for all the reasons mentioned it seems that we might end up needing one. But it feels also to me like it's an easy feature to add later on, correct? And if/when we do it could look like this: Can we try having also the "delete" button removing any block selected? |
Right, it's an easy to add feature. Maybe a "delete" button makes sense somewhere close to the block mover. |
Could work for sure. My reluctance to adding this feature isn't that I want us to not have it — I still believe it's where we'll end up. But I'm reluctant to optimize for this soon, because if we add the button, it's there forever. But if we don't add it, once we can start blogging with this plugin, dogfooding it, (which is any day now) we'll be able to know for sure whether we absolutely need this button. I think that answer will present itself rather quickly. Some of the alternatives I've looked at have this feature, others don't. The big kicker that Nicola mentioned previously is that it's very critical for the user experience that you know what you've selected — i.e. the block — at which point pressing the delete button or the backspace button would feel like a natural editor way of removing said piece. However since we have more complex blocks with input fields that take your literal focus, those keys will behave differently therefor mandating an explicit button. |
Post Settings can be toggled off, for a more immersive experience, and you should probably still have an affordance for deleting a block without that one enabled. |
I need to agree with @GaryJones. When adding a new block, I fumbled around for quite a while looking for a means of deleting the block. Particularly if you accidentally select the incorrect block type (eg: I didn't mean to click on smugmug), and want to get rid of it, there doesn't seem to be a means of deleting that block. In this instance, clicking delete on the keyboard is taking me back in my browser, away from the new post view. This seems to occur if the embed block is the first block in the post, rather than having a text block introducing the post. |
Just to add my thoughts here, I agree that adding some visual way to delete blocks is important.
|
* chore(package): update stylelint to version 7.0.0 (#83) * fix: Deprecated `no-missing-eof-newline` rule. Use the new `no-missing-end-of-source-newline` rule instead. (#84) * fix: Fixed font-family-name-quotes` test warning message in `values.js`. (#85) * feat: Add `property-no-unknown` rule. (#86) * Update install instructions to add `--save-dev` * chore(package): update stylelint to version 7.2.0 (#87) * chore(package): update AVA to version 0.16.0 (#88) * chore(package): update eslint-config-stylelint to version 4.0.0 (#89) * chore(package): update ESLint to version 3.0.0 (#90) * feat: Add `at-rule-no-unknown` rule. (#91) * feat: Add `selector-class-pattern` and `selector-id-pattern` rules. (#92) * Prepare 9.0.0 * Merge branch 'master' of github.com:ntwb/stylelint-config-wordpress * feat: Add SCSS preset config (#96) * feat: Add SCSS preset config * fix: Include README.md and scss.js in package.json files list * test: Add initial SCSS tests * Prepare 9.1.0 * Prepare 9.1.1 * refactor: Use ECMAScript 8/2017, use async/await instead of returning a promise and move css code out of tests to individual files * feat: Use ECMAScript 8/2017 * refactor: Use async/await instead of returning a promise and move css code out of tests to individual files * chore(package): update dependencies (#100) https://greenkeeper.io/ * chore: Add NodeJS 7 to Travis & AppVeyor CI test matrix's (#102) * chore(package): update stylelint to version 7.5.0 (#103) * chore: Add NodeJS 7.x changelog note * feat: Add `selector-no-empty` rule. (#104) * chore(package): Update `eslint-plugin-ava` to version 4.0.0 (#105) * fix: SCSS: Dissalow `@debug` at-rules. * fix: SCSS: Add `scss/selector-no-redundant-nesting-selector` rule. * Update ava to the latest version 🚀 (#109) * chore(package): update ava to version 0.17.0 * refactor: Include path to test fixtures https://greenkeeper.io/ * chore(package): update npm-run-all to version 4.0.0 (#111) https://greenkeeper.io/ * Update eslint-config-stylelint to the latest version 🚀 (#110) * chore(package): update eslint-config-stylelint to version 6.0.0 * refactor: Update tests per latest `eslint-config-stylelint` * chore: bump minimum NodeJS requirement to 6.9.1 and drop NodeJS 4.x (#118) * chore: Drop NodeJS from Travis CI * chore: Drop NodeJS from AppVeyor * chore: Bump minimum NodeJS requirement to 6.9.1 * chore: Update remark and remark plugin packages (#120) * Update ava to the latest version 🚀 (#112) * chore(package): update ava to version 0.18.0 https://greenkeeper.io/ * chore: update ava to version 0.19.1 * Updated and removed deprecated stylelint rules for 8.0 (#116) * Updated and removed deprecated stylelint rules * Removed deprecated unit test * chore: Add require NodeJS 6.x LTS changelog note * chore: Update changelog for changes made in #116 * refactor: Switch from AVA to Jest for tests. (#122) * tests: Fix `media-query-list-comma-space-before` tests * refactor: Switch from AVA to Jest for tests. * chore(package): update stylelint to version 7.10.1 (#123) * chore(package): update stylelint-scss to version 1.4.4 (#124) * chore(package): update eslint to version 3.19.0 (#125) * refactor: Switch from eslint-plugin-ava to eslint-plugin-jest. (#126) * refactor: Switch from eslint-plugin-ava to eslint-plugin-jest. * docs: Add changelog entry for switch from eslint-plugin-ava to eslint-plugin-jest * Fixed: Added `stylelint-scss` plugin @if/@else placement rules. (#127) * fix: Ignore proprietary `DXImageTransform.Microsoft` MS filters (#128) * docs: Update CHANEGLOG * Merge branch 'master' of github.com:ntwb/stylelint-config-wordpress * feat: Prepare `10.0.0` release. * fix: Remove stylelint v8 deprecated rule `rule-non-nested-empty-line-before` from SCSS config. (#130) * feat: Prepare `10.0.1` release. * fix: Add `@import` to `ignoreAtRules` option in `at-rule-empty-line-before` rule for SCSS config (#131) * feat: Prepare `10.0.2` release. * chore(package): update eslint-plugin-jest to version 20.0.0 (#134) * chore(package): update jest to version 20.0.0 (#135) * docs: Update docs to reflect the new repository home * docs: Update docs to reflect the new repository home at https://github.com/WordPress-Coding-Standards * chore: Update changelog noting repo location change * fix: Add `declaration-property-unit-whitelist` rule to enforce unitless `line-height` values. (#133) * fix: Include CSS config `at-rule-empty-line-before` options in SCSS config. (#139) * Fix: Allow `px` units in `line-height` values for the `declaration-property-unit-whitelist` rule. (#140) * feat: Prepare `11.0.0` release. * Updated README section links (#141) * chore: Add NodeJS v8 to Travis CI build matrix (#143) * chore: Switch from Node.js "current v7.x branch to v8.x for AppVeyor CI (#145) * chore: Drop Node.js v7.x branch from Travis CI (#146) * refactor: Switch to shared `recommended` config from `eslint-plugin-wordpress` for ESLint configuration. (#147) * refactor: Switch to shared `recommended` config from `eslint-plugin-wordpress` for ESLint configuration. * chore: Use `git://` protocol * chore: Include a commit hash * chore: Update ESLint to version 4.1.0 * chore: Update ESLint to version 4.x * refactor: Update indentation per updated ESLint version 4.x `indent` rule. * chore: Add a `dry-release` npm task (#149) * chore: Move ESLint config from `package.json` to `.eslintrc.json` file. (#152) * chore: Use the latest npm for all Node.js Travis CI jobs * chore: Add npm 5's `package-lock.json` file * chore: Add Greenkeeper support for npm 5.x's `package-lock.json` file. * tests: Add Jest snapshot tests * tests: Update Jest snapshots * chore(package): update stylelint to version 8.0.0 * fix: Add initial support for long comments in headers of WordPress theme `style.css` files. (#151) This change allows for longer comments in themes header: • URLs longer than 80 characters • Descriptions longer than 80 characters • Tags longer than 80 characters Fixes #150. * tests: Add some bbPress Jest snapshot tests. * tests: Update Jest SCSS tests to use SCSS config * tests: Update Jest SCSS test snapshots * tests: Add some BuddyPress Jest snapshot tests. * chore: Use the `runInBand` flag for Jest Travis CI jonbs * tests: Update snapshots * tests: Update tests to account for new rules introduced in `stylelint-config-recommended`. * chore: Add `stylelint-config-recommended` extend base configuration from it. * docs: Update CHANGELOG with `stylelint-config-recommended` changes * docs: Remove the styleguide from the repo. We should have one canonical source of truth and not try to maintain two instances. https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/ * refactor: Use `scss/at-rule-no-unknown` in `scss` shared config. * feat: Prepare `12.0.0` release. * chore(package): update remark-cli to version 4.0.0 * chore: Update `package-lock.json` * chore(package): update remark-preset-lint-recommended to version 3.0.0 * chore: Update `package-lock.json` * fix(package): update stylelint-scss to version 2.0.0 * chore: Update `package-lock.json` * chore: Add `stylelint-find-rules` * chore(package): update stylelint-find-rules to version 1.0.1 Closes #168 * chore: Update package-lock.json * chore: Updated `stylelint` peer dependency version to `^8.0.0`. * chore(package): update jest to version 21.0.0 * chore: Update `package-lock.json` * chore(package): update eslint-plugin-jest to version 21.0.0 * chore: Update `package-lock.json` * tests: Remove Jest snapshots (#176) * docs: Update CHANGELOG * Use toHaveLength() in tests Not only does it check the length of something is exactly a certain integer, it also checks that the length property exists in the first place. Addresses lint issues identified at https://www.bithound.io/github/WordPress-Coding-Standards/stylelint-config-wordpress/e2bbe0d9c867cc95da6de5b3bff2a73742135fb6/files#failing * chore: Remove Greenkeeper lock file configuration from `.travis.yml` * chore: Remove `package-lock.json` * chore: Add `package-lock.json` to `.gitignore` * chore: Add `.npmrc` file to prevent npm creating a `packake-lock.json` file. * chore(package): update jest to version 22.0.0 * chore: update `.editorconfig` per upstream WordPress' `.eitorconfig` See https://core.trac.wordpress.org/browser/trunk/.editorconfig * chore: use tabs for indentaion in `package.json` per WordPress coding standards * chore: use tabs for indentaion in `.eslintrc.json` per WordPress coding standards * chore: use `* text=auto` in `.gitattributes` * chore(package): update remark-cli to version 5.0.0 * chore: standardize Jest tests * chore: add commitlint * chore: bump minimum Nod.js required version to `8.9.3` * test: improved `no-duplicate-selectors` tests * feat: update `stylelint` to `9.1.3` * chore: updated `stylelint-config-recommended` to `2.1.0` * chore: updated: `stylelint-scss` to `2.1.0` * feat: update `selector-pseudo-element-colon-notation` to use `double` * feat: prepare `13.0.0` release * fix(package): update stylelint-scss to version 3.0.0 * docs: update changelog * test: add SCSS tests for _extends_ shared configs This test ensures that the rules included in the _shared configs_ inherited by this SCSS _shared config_ via the `extends` option are in actual fact included. The `stylelint-config-wordpress/scss` shared config _extends_ the `stylelint-config-wordpress` shared config, which in turn _extends_ the `stylelint-config-recommended` shared config. * docs: update changelog * test: standardize invalid tests warnings test name verbiage * test: use Jest snapshots for invalid tests This change simplifies the maintainence of the invalid CSS and SCSS tests * feat: the `/scss` config now extends `stylelint-config-recommended-scss` * feat: update `stylelint` to `9.2.0` * docs: add basic _extends_ shared config references * Update to node 10 in .travis.yml * Drop Node.js v9.x * Update appveyor.yml * chore(package): update @commitlint/cli to version 7.0.0 * chore(package): update @commitlint/config-conventional to version 7.0.1 (#203) Closes #201 * chore(package): update npmpub to version 4.0.1 Closes #204 * feat: update stylelint to `9.5.0` * chore: update ESLint to `5.4.0` * chore: update Jest to `23.5.0` * chore: update `eslint-plugin-jest` to `21.21.0` * chore: update `npmpub` to `4.1.0` * chore: update `npm-run-all` to `4.1.3` * chore: update `stylelint-find-rules` to `1.1.1` * chore: update remark presets: • `remark-preset-lint-consistent` to `2.0.2` • `remark-preset-lint-recommended` to `3.0.2` * feat: update `stylelint-scss` to `3.3.0` * chore: add `npm-package-json-lint` * chore: add `@wordpress/npm-package-json-lint-config` * chore: update `package.json` property order * feat: Prepare `13.1.0` release (#208) * chore(package): update husky to version 1.1.2 (#210) Closes #209 * chore(package): update remark-cli to version 6.0.0 (#211) * chore(package): update eslint-plugin-jest to version 22.0.0 (#212) * Update stylelint-find-rules to the latest version 🚀 (#213) ## The devDependency [stylelint-find-rules](https://github.com/alexilyaev/stylelint-find-rules) was updated from `1.1.1` to `2.0.0`. This version is **not covered** by your **current version range**. If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update. * chore(package): update stylelint to version 10.0.1 (#215) Closes #214 * chore: update @commitlint * chore: update `stylelint-config-recommended` to v2.2.0 * chore: update `stylelint-scss` to v3.6.0 * chore: update devDependencies * feat: prepare `14.0.0` release * chore(package): update husky to version 2.2.0 Closes #219 * chore: update `husky.hooks` config in `package.json` * chore(package): update @wordpress/npm-package-json-lint-config to version 2.0.0 * chore(package): update husky to version 3.0.1 Closes #227 * test: fix type in snapshot test * chore(package): update @commitlint/cli to version 8.0.0 * chore(package): update @commitlint/config-conventional to version 8.0.0 * chore(package): update remark-cli to version 7.0.0 * chore(package): update packages to latest versions * chore(package): restore peerDependencies stylelint versions * chore(node): bump minimum Node.JS to LTS version 10.x * ci: use `npm test` to include lint tasks in CI jobs * docs: nocapital_S_dangit * chore(package): update npmpub to version 5.0.0 * fix(package): update stylelint-config-recommended to version 3.0.0 * chore(package): update stylelint to version 11.0.0 * chore: update `peerDependencies` for stylelint 11.0.0 * docs: update changelog * docs: fix stylelint removed compat version * Update stylelint-config-recommended-scss to the latest version 🚀 (#238) * Update eslint to the latest version 🚀 (#233) * chore(package): update eslint to version 6.3.0 * chore: add `ecmaVersion: 2015,` to `parserOptions` ESLint config * docs: update changelog * chore: bump dependencies * chore: add Node.js v12 to Travis CI test matrix, remove 8.x, min 10.x chore: add Node.js v12 to Travis CI test matrix * feat: prepare `15.0.0` release * Merge pull request #241 from WordPress-Coding-Standards/update/find-rules chore: update `stylelint-find-rules` to 2.2.0 * Update npm-package-json-lint to the latest version 🚀 (#242) Update npm-package-json-lint to the latest version 🚀 * Use `@wordpress/scripts` (#231) Use `@wordpress/scripts` * chore: update `stylelint` to 12.0.0 * chore: update `stylelint-scss` to 3.13.0 * chore: update `stylelint-config-recommended-scss` to 4.1.0 * chore: update `husky` to 3.1.0 * chore: update `remark-cli` to 7.0.1 * chore: update `@wordpress/scripts` to 6.0.0 * Fixed: `selector-class-*` regex to account for numerals, case de… (#247) Fixed: `selector-class-*` regex to account for numerals, case detection, and ensure kebab-case * ci: add Windows to Travis CI (#248) ci: add Windows to Travis CI * chore: remove AppVeyor (#249) chore: remove AppVeyor * feat: prepare `16.0.0` release * test: update comment fof scss-invalid test to eslint-jsdoc warnings * feat: prepare `16.0.0` release * fix: npm script temp workaround * Merge pull request #251 from WordPress-Coding-Standards/greenkeeper/@wordpress/scripts-6.1.1 chore(package): update @wordpress/scripts to version 6.1.1 * Revert "fix: npm script temp workaround" This reverts commit e409112e0e8f3965993e3f1b3a6ecc3c8ffdc8e0. * Update husky to the latest version 🚀 (#252) Update husky to the latest version 🚀 * Merge pull request #255 from WordPress-Coding-Standards/greenkeeper/stylelint-13.1.0 Update stylelint to the latest version 🚀 * Merge pull request #254 from WordPress-Coding-Standards/greenkeeper/@wordpress/scripts-7.0.0 Update @wordpress/scripts to the latest version 🚀 * chore(package): update @wordpress/scripts to version 7.1.2 * chore(package): update stylelint-scss to version 3.14.2 * chore(package): update stylelint-config-recommended-scss to version 4.2.0 * chore(package): update husky to version 4.2.3 * chore(package): update @commitlint/cli to version 8.3.5 * chore(package): update @commitlint/config-conventional to version 8.3.4 * Merge pull request #256 from WordPress-Coding-Standards/greenkeeper/remark-cli-8.0.0 Update remark-cli to the latest version 🚀 * Merge pull request #258 from WordPress-Coding-Standards/greenkeeper/remark-preset-lint-recommended-4.0.0 Update remark-preset-lint-recommended to the latest version 🚀 * Merge pull request #257 from WordPress-Coding-Standards/greenkeeper/remark-preset-lint-consistent-3.0.0 Update remark-preset-lint-consistent to the latest version 🚀 * Merge pull request #260 from WordPress-Coding-Standards/greenkeeper/@wordpress/scripts-8.0.1 * Merge pull request #264 from WordPress-Coding-Standards/greenkeeper/@wordpress/scripts-10.0.0 chore(package): update @wordpress/scripts to version 10.0.0 * chore: update `stylelint-scss` to 3.17.2 * feat: prepare `17.0.0` release * refactor: rename `stylelint-config-wordpress` to `stylelint-config` * refactor: rename `stylelint-config-wordpress` to `@wordpress/stylelint-config` in `package.json` * refactor: remove `@commitlint` from `@wordpress/stylelint-config` * refactor: remove `husky` from `@wordpress/stylelint-config` * refactor: trim npm package keywords from `@wordpress/stylelint-config` * refactor: remove `.editorconfig` from `@wordpress/stylelint-config` * refactor: remove `npmpub` from `@wordpress/stylelint-config` * refactor: remove `eslintConfig` from `@wordpress/stylelint-config` * refactor: remove `remarkConfig` from `@wordpress/stylelint-config` * refactor: remove `remark` from `@wordpress/stylelint-config` * refactor: remove `engines` from `@wordpress/stylelint-config` * refactor: remove `prettier.config.js` from `@wordpress/stylelint-config` * refactor: remove `.gitattributes` from `@wordpress/stylelint-config` * refactor: remove `.gitignore` from `@wordpress/stylelint-config` * refactor: remove `.travis.yml` from `@wordpress/stylelint-config` * tests: refactor tests in `@wordpress/stylelint-config` * chore: add `.stylelintignore` for _invalid_ scss test fixtures in `@wordpress/stylelint-config` * Revert "chore: add `.stylelintignore` for _invalid_ scss test fixtures in `@wordpress/stylelint-config`" This reverts commit 05ab441. * chore: add `.stylelintrc.json` for test fixtures in `@wordpress/stylelint-config` * tests: update Jest snapshots in `@wordpress/stylelint-config` * docs: update `docs/manifest.json` for `@wordpress/stylelint-config` * chore: update `@wordpress/stylelint-config` package description * chore: remove `devDependencies` from `@wordpress/stylelint-config` * chore: remove package `scripts` from `@wordpress/stylelint-config` * chore: remove superfluous `.stylelintignore` from `@wordpress/stylelint-config` * chore: add Lerna `publishConfig` to `@wordpress/stylelint-config` * Update Jest snapshots * Backfill release dates in changelog * Update package author * Update readme * Update comment to use renamed package name * Rename tests to match other packages * Remove stylelint `^10.1.0`, `^11.0.0`, and `^12.0.0` as peer dependency. Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com> Co-authored-by: Harley Oliver <[email protected]> Co-authored-by: Heather B <[email protected]> Co-authored-by: Gary Jones <[email protected]> Co-authored-by: greenkeeper[bot] <23040076+greenkeeper[bot]@users.noreply.github.com> Co-authored-by: Dominik Schilling <[email protected]>
It is currently not possible to delete a block without having to select the contents. A delete button has been added to the single instance TinyMCE prototype. It will replace the contents with an empty slot and set the selection there.
https://wordpress.github.io/gutenberg/tinymce-single/
The text was updated successfully, but these errors were encountered: