-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix: Categories Block: hierarchical Dropdown #13567
Conversation
@@ -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 ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @gziolo, I applied the refactor to use a functional code style. Thank you for your suggestion!
ca7ffb5
to
2853d14
Compare
@@ -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( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This made me think we could filter inside getCategories
method to make it easier to follow here.
I will test this PR by tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice, thanks for investigating further, testing your changes soon.
2853d14
to
c0de3c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look great after 2 iterations 😃
It tests well, I can confirm that issue is resolved.
Thank you for the reviews @gziolo! |
…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
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 commit fixes that problem.
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 commit fixes that problem.
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:
After: