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

Fix: Categories Block: hierarchical Dropdown #13567

Merged
merged 1 commit into from Jan 31, 2019

Conversation

Projects
None yet
2 participants
@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Jan 29, 2019

Description

Fixes: #13560

Previously higher than second levels categories here rendered multiple times on the categories block, when "Show Hierarchy" and "Display as Dropdown" options here true.
This PR fixes that problem.

How has this been tested?

I created a Level 1 category without parent category.
I created a Level 2 category child of Level 1.
I created a Level 3 category child of Level 2.
I added a post with a Level 3 category.
I added the Category block and enable "Show Hierarchy" and "Display as Dropdown".
I checked the dropdown as displayed as expected without Level 2 and Level 3 being repeated.

Screenshots

Before:
screenshot 2019-01-29 at 18 56 20
After:
screenshot 2019-01-29 at 18 55 24

@@ -116,7 +116,11 @@ class CategoriesEdit extends Component {
{ __( 'Categories' ) }
</label>
<select id={ selectId } className="wp-block-categories__dropdown">
{ categories.map( ( category ) => this.renderCategoryDropdownItem( category, 0 ) ) }
{ categories.map( ( category ) => {

This comment has been minimized.

@gziolo

gziolo Jan 30, 2019

Member

This is the case where you can also use more functional approach and combine filter and map:

{ categories.filter( ( { parent } ) => parent === 0 ).map( ( category ) => this.renderCategoryDropdownItem( category, 0 ) ) }

Still the same code, just coded differently and I haven't tested :)

This comment has been minimized.

@jorgefilipecosta

jorgefilipecosta Jan 30, 2019

Author Member

Hi @gziolo, I applied the refactor to use a functional code style. Thank you for your suggestion!

@jorgefilipecosta jorgefilipecosta force-pushed the fix/categories-block-hierarchical-Dropdown branch from ca7ffb5 to 2853d14 Jan 30, 2019

@@ -116,7 +116,13 @@ class CategoriesEdit extends Component {
{ __( 'Categories' ) }
</label>
<select id={ selectId } className="wp-block-categories__dropdown">
{ categories.map( ( category ) => this.renderCategoryDropdownItem( category, 0 ) ) }
{ categories.filter(

This comment has been minimized.

@gziolo

gziolo Jan 30, 2019

Member

This made me think we could filter inside getCategories method to make it easier to follow here.

I will test this PR by tomorrow.

This comment has been minimized.

@jorgefilipecosta

jorgefilipecosta Jan 30, 2019

Author Member

I made a new check in the logic of the code and it looks like the filtering was already being accounted in the logic. The existing logic only had a bug when accessing a prop that caused the filtering to not be performed. The fix was simplified.

This comment has been minimized.

@gziolo

gziolo Jan 31, 2019

Member

This is nice, thanks for investigating further, testing your changes soon.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/categories-block-hierarchical-Dropdown branch from 2853d14 to c0de3c2 Jan 30, 2019

@gziolo gziolo added this to the 5.0 (Gutenberg) milestone Jan 31, 2019

@gziolo

gziolo approved these changes Jan 31, 2019

Copy link
Member

gziolo left a comment

Code changes look great after 2 iterations 😃
It tests well, I can confirm that issue is resolved.

@jorgefilipecosta jorgefilipecosta merged commit a8dcfb2 into master Jan 31, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jorgefilipecosta jorgefilipecosta deleted the fix/categories-block-hierarchical-Dropdown branch Jan 31, 2019

@jorgefilipecosta

This comment has been minimized.

Copy link
Member Author

jorgefilipecosta commented Jan 31, 2019

Thank you for the reviews @gziolo!

daniloercoli added a commit that referenced this pull request Feb 1, 2019

Merge branch 'master' of https://github.com/WordPress/gutenberg into …
…rnmobile/372-use-RichText-on-Title-block

* 'master' of https://github.com/WordPress/gutenberg:
  Try alternate list item jump fix. (#12941)
  Mobile bottom sheet component (#13612)
  Remove unintentional right-margin on last odd-item. (#12199)
  Introduce left and right float alignment options to latest posts block (#8814)
  Fix Google Docs table paste (#13543)
  Increase bottom padding on gallery image caption (#13623)
  Fix the editor save keyboard shortcut not working in code editor view (#13159)
  Plugin: Deprecate gutenberg_add_admin_body_class (#13572)
  Rnmobile/upload media failed state (#13615)
  Make clickOnMoreMenuItem not dependent on aria labels (#13166)
  Add: className prop support to server side render. (#13568)
  Fix: Categories Block: hierarchical Dropdown (#13567)
  Docs: Add clarification about git workflow (#13534)
  Plugin: Remove `user_can_richedit` filtering (#13608)
  eslint-plugin: Add rule `no-unused-vars-before-return` (#12828)
  Image settings button (#13597)
  Fixed wording for the color picker saturation (#13479)

# Conflicts:
#	packages/block-library/src/image/edit.native.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment