Fix: make meta+A behaviour of selecting all blocks work on safari and firefox#8180
Conversation
5a320d4 to
4503511
Compare
4503511 to
0372e34
Compare
noisysocks
left a comment
There was a problem hiding this comment.
I've tested this in Chrome, Firefox, Safari, Edge, and IE 11 and the bug is fixed 👍
Might be good to get another pair of eyes on this since I'm not 100% on how it works.
| // but on some browsers (safari and firefox) calling isEntirelySelected right way still returns false. | ||
| // So to make sure we are compatible with this browsers we set is entirely selected to true | ||
| // after the first meta+a keypress assuming TinyMCE will make all content entirely selected. | ||
| this.isEntirelySelected = target.isContentEditable ? true : isEntirelySelected( target ); |
There was a problem hiding this comment.
I'm trying to wrap my head around this logic. When would primary+a not result in all of the text being selected? In other words, why can't this line be:
this.isEntirelySelected = true;There was a problem hiding this comment.
I'm very hesitant of anything which forks logic based on browser differences. Can we simplify to what @noisysocks said? Should that live in isEntirelySelected ? Do we need isEntirelySelected at all? What is it about being isContentEditable that makes us assume it's entirely selected?
| // but on some browsers (safari and firefox) calling isEntirelySelected right way still returns false. | ||
| // So to make sure we are compatible with this browsers we set is entirely selected to true | ||
| // after the first meta+a keypress assuming TinyMCE will make all content entirely selected. | ||
| this.isEntirelySelected = target.isContentEditable ? true : isEntirelySelected( target ); |
There was a problem hiding this comment.
I'm very hesitant of anything which forks logic based on browser differences. Can we simplify to what @noisysocks said? Should that live in isEntirelySelected ? Do we need isEntirelySelected at all? What is it about being isContentEditable that makes us assume it's entirely selected?
0372e34 to
5e4d8dc
Compare
|
Hi @aduth, @noisysocks I checked that in fact, we could simplify the logic to what @noisysocks said. |
I think the feedback was addressed and the code was simplified to the solution proposed by @noisysocks.
… firefox. On safari and firefox isEntirelySelected( target ) called right after meta+A event is fired without releasing the meta key in the middle returns false but content gets selected anyway. If the target is editable it is safe to assume TinyMCE will make sure all content gets selected so in this cases we set the value to true without calling isEntirelySelected.
5e4d8dc to
56819c6
Compare
…rnmobile/port-quote-block-step-1 * 'master' of https://github.com/WordPress/gutenberg: (22 commits) Add removed periods to block descriptions. (#11367) Remove findDOMNode usage from the inserter (#11363) Remove deprecated componentWillReceiveProps from TinyMCE component (#11368) Create file blocks when dropping multiple files at once (#11297) Try avoiding the deprecated findDOMNode API from DropZone Provider (#11168) Fix: make meta+A behaviour of selecting all blocks work on safari and firefox. (#8180) Remove _wpGutenbergCodeEditorSettings and wp.codeEditor assets (#11342) Remove the Cloudflare warning (#11350) Image Block: Use source_url for media file link (#11254) Enhance styling of nextpage block using the Hr element (#11354) Embed block refactor and tidy (#10958) Nonce Middleware: Wrap the nonce middleware function into it's own function that isn't regenerated on every API request. (#11347) Fix RTL block alignments (#11293) RichText: fix buggy enter/delete behaviour (#11287) Remove code coverage setup (#11198) Parser: Runs all parser implementations against the same tests (#11320) Stop trying to autosave when title and classic block content both are empty. (#10404) Fix "Mac OS" typo + use fancy quotes consistently (#11310) Update documentation link paths (#11324) Editor: Reshape blocks state under own key (#11315) ... # Conflicts: # gutenberg-mobile
On safari and firefox isEntirelySelected( target ) called right after meta+A event is fired when meta key is not released in the middle returns false but the content gets selected anyway.
If the target is editable it is safe to assume TinyMCE will make sure all content gets selected so in this cases we set the value to true without calling isEntirelySelected.
Fixes: #7445
How has this been tested?
I added some paragraphs.
On safari and firefox, I checked that when we press meta+A in paragraph two times all the blocks get selected.