Skip to content

Conversation

@ephox-mogran
Copy link
Contributor

@ephox-mogran ephox-mogran commented Oct 12, 2017

Description

NOTE: Only merging to an existing branch, not master

Building on #2960, just adding the ability for pressing ESC to refocus the block (in whatever way the particular block receives focus)

How Has This Been Tested?

Just manually.

Screenshots (jpeg or gifs if applicable):

Types of changes

Bug fix for ESC to focus editable areas

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation. (no documentation was there)

@ephox-mogran
Copy link
Contributor Author

@youknowriad I've noticed that pressing ESC in the toolbar for image blocks has no effect. Is that intended?

@codecov
Copy link

codecov bot commented Oct 12, 2017

Codecov Report

Merging #2996 into try/toolbar-arrow-navigation will decrease coverage by 0.51%.
The diff coverage is 0%.

Impacted file tree graph

@@                       Coverage Diff                        @@
##           try/toolbar-arrow-navigation    #2996      +/-   ##
================================================================
- Coverage                         34.38%   33.87%   -0.52%     
================================================================
  Files                               197      194       -3     
  Lines                              5935     5798     -137     
  Branches                           1065     1021      -44     
================================================================
- Hits                               2041     1964      -77     
+ Misses                             3283     3240      -43     
+ Partials                            611      594      -17
Impacted Files Coverage Δ
editor/modes/visual-editor/block.js 0% <ø> (ø) ⬆️
editor/block-toolbar/index.js 0% <0%> (ø) ⬆️
blocks/api/paste/utils.js 82.97% <0%> (-13.23%) ⬇️
blocks/api/paste/index.js 86.66% <0%> (-1.8%) ⬇️
blocks/editable/tinymce.js 0% <0%> (ø) ⬆️
blocks/api/paste/create-unwrapper.js 100% <0%> (ø) ⬆️
editor/writing-flow/index.js 0% <0%> (ø) ⬆️
editor/index.js 0% <0%> (ø) ⬆️
blocks/api/paste/table-normaliser.js
blocks/api/paste/inline-content-converter.js
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b8aec3...683555b. Read the comment docs.

@youknowriad
Copy link
Contributor

Like discussed in DM, this approach doesn't work seamlessly in all the blocks because we're not making a strong difference between the "selected" and the "focused" state in the store. I think we should keep the current approach for now, and try to bring this focused/selected state in a separate PR.

@youknowriad youknowriad deleted the try/toolbar-arrow-navigation-B branch October 13, 2017 06:59
@ephox-mogran
Copy link
Contributor Author

@youknowriad has

we're not making a strong difference between the "selected" and the "focused" state in the store

started to be resolved anywhere?

@youknowriad
Copy link
Contributor

I know @mcsf had thoughts about this. I don't know if he started something or not. Also, the vertical navigation PR might be related.

@mcsf
Copy link
Contributor

mcsf commented Oct 24, 2017

Nothing yet, no. Just loose thoughts in my head. Selection vs. focus is a thing to address after vertical nav (#2988), I believe.

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.

4 participants