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

Navigation: Fix warning when stretch justification is used #50568

Merged
merged 1 commit into from
May 14, 2023

Conversation

andrewserong
Copy link
Contributor

@andrewserong andrewserong commented May 12, 2023

What?

Fixes the warning issue reported in #50550

In the Navigation block's PHP, check that the provided justification value is present in the mapping array before attempting to access it.

Why?

Since stretch was introduced as a valid value, when stretch is set on the Navigation block, it throws a warning when rendered on the site frontend.

This PR doesn't implement proper support for stretch in this block — it's just looking at resolving this PHP warning.

How?

  • Check the value exists before attempting to access it

Testing Instructions

  1. Add a Navigation block
  2. Set it to vertical orientation
  3. Select stretch content justification (the right-most option)
  4. Save the block
  5. Load the post / page / site on the site's frontend
  6. In PHP logs, note that with this PR applied there should be no warning logged.
  7. stretch doesn't really do anything for this block yet (I don't think), but double-check that left, centre, right justifications still work as expected

Testing Instructions for Keyboard

Screenshots or screencast

This is the warning that should not be logged with this PR applied:

image

@andrewserong andrewserong added [Type] Bug An existing feature does not function as intended [Block] Navigation Affects the Navigation Block [Feature] Layout Layout block support, its UI controls, and style output. labels May 12, 2023
@andrewserong andrewserong self-assigned this May 12, 2023
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

Does what it says on the tin.
LGTM 👍

@andrewserong
Copy link
Contributor Author

Thanks for the review!

@andrewserong andrewserong merged commit 4af25a1 into trunk May 14, 2023
@andrewserong andrewserong deleted the fix/navigation-block-array-key-warning branch May 14, 2023 22:40
@github-actions github-actions bot added this to the Gutenberg 15.9 milestone May 14, 2023
@tellthemachines
Copy link
Contributor

Thanks for fixing this! I've been meaning to update the layout logic in the Nav block since #44600, as we shouldn't need the custom properties any longer, but there's always so much to do 😅

@andrewserong
Copy link
Contributor Author

but there's always so much to do 😅

Oh, I hear ya! My to-do list grows faster than my to-done list 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Feature] Layout Layout block support, its UI controls, and style output. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants