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 Archives block alignment and custom class bugs #9696

Merged
merged 1 commit into from Sep 12, 2018

Conversation

Projects
None yet
2 participants
@ZebulanStanphill
Contributor

ZebulanStanphill commented Sep 7, 2018

Description

This PR fixes two issues:

  • The Archives block uses an alignnone class when no alignment is set, which is inconsistent with other blocks.
  • Custom class names do not work with the Archives block.

It also contains some minor code cleanup, reorganizing attributes alphabetically and using Array.prototype.includes to check for valid alignments (which is something the Latest Comments block already does), rather than the more verbose and harder-to-read series of ||s.

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Sep 7, 2018

Contributor

I can't figure out what is going wrong with the tests. Any ideas?

Contributor

ZebulanStanphill commented Sep 7, 2018

I can't figure out what is going wrong with the tests. Any ideas?

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Sep 7, 2018

Contributor

Fixed the tests. Thanks for the help, @aduth! 🙂

Contributor

ZebulanStanphill commented Sep 7, 2018

Fixed the tests. Thanks for the help, @aduth! 🙂

@ZebulanStanphill

This comment has been minimized.

Show comment
Hide comment
@ZebulanStanphill

ZebulanStanphill Sep 10, 2018

Contributor

Squashed and rebased. Ready for review.

Contributor

ZebulanStanphill commented Sep 10, 2018

Squashed and rebased. Ready for review.

'align' => array(
'type' => 'string',
),
'className' => array(

This comment has been minimized.

@youknowriad

youknowriad Sep 12, 2018

Contributor

Not certain declaring this one is necessary here because there's a hook adding this attribute. Though one could think the hook is only declared on JavaScript which means nothing guarantees that it's available in PHP (which hopefully the server-side awareness work should resolve)

@youknowriad

youknowriad Sep 12, 2018

Contributor

Not certain declaring this one is necessary here because there's a hook adding this attribute. Though one could think the hook is only declared on JavaScript which means nothing guarantees that it's available in PHP (which hopefully the server-side awareness work should resolve)

@youknowriad

LGTM 👍 Thanks for the tweaks

@youknowriad youknowriad added this to the 3.9 milestone Sep 12, 2018

@youknowriad youknowriad added the Blocks label Sep 12, 2018

@youknowriad youknowriad merged commit 77ff8e6 into WordPress:master Sep 12, 2018

2 checks passed

codecov/project 49.02% remains the same compared to 2fa3d3d
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ZebulanStanphill ZebulanStanphill deleted the ZebulanStanphill:fix/archives-block-alignment-and-custom-classes branch Sep 12, 2018

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