Skip to content

Conversation

youknowriad
Copy link
Contributor

closes #9807

This PR does two things:

@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Sep 12, 2018
@youknowriad youknowriad self-assigned this Sep 12, 2018
@youknowriad youknowriad requested review from jasmussen and a team September 12, 2018 09:53
Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dig it. Tested locally and it was nice.

Seems a good contender for an e2e test or two. If you want to add them now I'm happy to review again, otherwise could you file an issue? 😄

return (
<NavigableToolbar
className="edit-post-header-toolbar"
aria-label={ __( 'Editor Toolbar' ) }
>
<FullscreenModeClose />
<div>
<Inserter position="bottom right" />
<Inserter disabled={ mode !== 'visual' } position="bottom right" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These sorts of magic constants always irk me, even when they're strings. This is pretty obvious in its intent, but it's still possible to make typos, etc. 🤷‍♂️

Not really isolated to this PR though.

@jasmussen
Copy link
Contributor

Yep! Ship it. This works well.

@youknowriad
Copy link
Contributor Author

@tofumatt Test added if you want to take another look.

Copy link
Member

@tofumatt tofumatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the test! ❤️

🚢

switchToEditor,
} from '../support/utils';

describe( 'code editor', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have an editor-modes.test.js that this might fit into better, up to you though. Our test names are odd 🤷‍♂️


describe( 'code editor', () => {
beforeEach( async () => {
await await newPost();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an echo in here? 😉

@youknowriad youknowriad force-pushed the update/tweaks-to-code-mode branch from 75bdcf9 to 306ba2e Compare September 12, 2018 11:49
@youknowriad youknowriad merged commit 555f656 into master Sep 12, 2018
@youknowriad youknowriad deleted the update/tweaks-to-code-mode branch September 12, 2018 12:22
@mcsf
Copy link
Contributor

mcsf commented Sep 12, 2018

Yay, less is more! I'm glad we took this route to fix the parent issue.

@mtias mtias added this to the 3.9 milestone Sep 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding block in Code Editor mode produces placeholder markup
5 participants