Skip to content

Fix: Navigation block: Make function names specific#20039

Merged
jorgefilipecosta merged 1 commit intomasterfrom
fix/navigation-block-make-function-names-specific
Feb 5, 2020
Merged

Fix: Navigation block: Make function names specific#20039
jorgefilipecosta merged 1 commit intomasterfrom
fix/navigation-block-make-function-names-specific

Conversation

@jorgefilipecosta
Copy link
Copy Markdown
Member

Description

The navigation block is defining functions whose name looks generic like build_css_colors.
This PR updates the names of functions in the navigation block to make them specific to the block.
In response to the comment raised by @TimothyBJacobs in trac ticket https://core.trac.wordpress.org/ticket/49348#comment:6. Thank you for catching this issue.

How has this been tested?

I verified the navigation block still works as before.

@jorgefilipecosta
Copy link
Copy Markdown
Member Author

@TimothyBJacobs do you think the new function names are adequate? Thank you in advance for any feedback you could provide.

@TimothyBJacobs
Copy link
Copy Markdown
Member

Those names make sense to me!

@jorgefilipecosta jorgefilipecosta added this to the Gutenberg 7.4 milestone Feb 4, 2020
@retrofox retrofox self-requested a review February 4, 2020 22:15
Copy link
Copy Markdown
Contributor

@retrofox retrofox left a comment

Choose a reason for hiding this comment

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

Tested the whole implementation of the <Navigation /> and it works as expected. 👍

@jorgefilipecosta jorgefilipecosta removed this from the Gutenberg 7.4 milestone Feb 5, 2020
@jorgefilipecosta jorgefilipecosta merged commit 2c587e5 into master Feb 5, 2020
@jorgefilipecosta jorgefilipecosta deleted the fix/navigation-block-make-function-names-specific branch February 5, 2020 07:42
@github-actions github-actions bot added this to the Gutenberg 7.5 milestone Feb 5, 2020
@aduth
Copy link
Copy Markdown
Member

aduth commented Feb 7, 2020

Related previous effort (and discussion): #18589

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants