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

Inserter: Show messaging when no blocks found #4040

Merged
merged 1 commit into from
Jan 2, 2018

Conversation

aduth
Copy link
Member

@aduth aduth commented Dec 15, 2017

This pull request seeks to add messaging when searching using the inserter reveals no available block options.

Before After
Before After

Testing instructions:

Verify that if no blocks are found by searching the inserter menu that a "No blocks found" message is shown instead.

@aduth aduth added Needs Design Feedback Needs general design feedback. [Feature] Inserter The main way to insert blocks using the + button in the editing interface labels Dec 15, 2017
Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

The code looks good to me and things are working as expected.

@@ -255,6 +256,14 @@ export class InserterMenu extends Component {
}

renderCategories( visibleBlocksByCategory ) {
if ( isEmpty( visibleBlocksByCategory ) ) {
Copy link
Member

@jorgefilipecosta jorgefilipecosta Dec 15, 2017

Choose a reason for hiding this comment

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

If the only usage of this logic is when searching it may make sense to make this verification in line 358 where we call render categories for search as it makes things more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think that makes sense. It's a bit complicated with trying to render the result of this.getVisibleBlocksByCategory inline, since we'd need to both test its non-emptiness and pass as the argument to renderCategories (or call twice, which is wasteful).

Lodash's _.cond could handle this pretty nicely, though maybe a bit less obvious to read?

cond( [
	[ isEmpty, () => (
		<span className="editor-inserter__no-results">
			{ __( 'No blocks found' ) }
		</span>
	) ],
	[ () => true, this.renderCategories ]
] )( this.getVisibleBlocksByCategory( this.getBlockTypes() ) );

Copy link
Member

Choose a reason for hiding this comment

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

This version with lodash cond is nice, but it is not obvious to read as you pointed out. The other possibility would save getVisibleBlocksByCategory in a const before the render but this way we may be computing it without need. I did not think about the complication because of being inline. It looks like our options have their downsides are not better than the current version we have, so I think we can ignore this detail and merge as it is right now.

@aduth aduth merged commit 266247d into master Jan 2, 2018
@aduth aduth deleted the update/inserter-no-blocks-found branch January 2, 2018 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Inserter The main way to insert blocks using the + button in the editing interface Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants