-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add Archives block #7949
Add Archives block #7949
Conversation
82008dd
to
31bccd8
Compare
core-blocks/archives/block.js
Outdated
this.toggleDisplayAsDropdown = this.toggleDisplayAsDropdown.bind( this ); | ||
} | ||
|
||
toggleShowPostCounts() { |
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.
Can we use just one toggle function for both attributes? See: https://github.com/WordPress/gutenberg/pull/7941/files#diff-4a7082059a72e724bea52a0a00c65773R35
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've changed this now to a functional component in ad5aa1d as per previous feedback on the old PR, so I don't think making them a single function would be workable now. On the plus side they're both one-liners now. :)
core-blocks/archives/index.php
Outdated
} | ||
|
||
$block_content = sprintf( | ||
'<div class="%1$s">%2$s</div>', |
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.
div
should be an 'ul'
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.
Nice catch! It should be a ul
when rendered as a list, and I've kept it as a div when rendered as a dropdown - 65025c1
Co-authored-by: Pascal Birchler <hello@pascalbirchler.ch> Co-authored-by: Miina Sikk <miina.sikk@gmail.com> Co-authored-by: Weston Ruter <weston@xwp.co>
…tor view Use of `pointer-events` to disable interaction was problematic. It didn't disabled the dropdown version of the block and it doesn't prevent keyboard interaction. The Disabled component handles these situations.
31bccd8
to
ad5aa1d
Compare
Hey @noisysocks & @mcsf - I think I've addressed all the review feedback from #5495 now. Would be great if one of you could look over may changes to make sure they're ok. Thanks! |
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.
Tests well, looks good. Thanks for working on this, @talldan.
export const settings = { | ||
title: __( 'Archives' ), | ||
|
||
description: __( 'Display a monthly archive of your site’s Posts.' ), |
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.
FWIW, filter widget_archives_dropdown_args
can change the periodic interval, meaning "monthly" may become inaccurate. However, I could see filtering as a more advanced feature, and sticking with "monthly" offers the benefit of conveying the basic idea of the block more clearly than e.g. "Display a periodic archive […]". Because of this, I don't mind sticking with the current copy.
This PR is a continuation of #5495, moved to a local branch for easier collaboration. Props @miina for the bulk of the implementation.
Uses #5602
Continued archives block implementation from #1518 and #5495. Archive block implementation that uses similar output and filters as the archives widget. Has alignment toolbar within the block (
left
,center
,right
).Screenshots:
Block toolbar:
Inspector toolbar:
Dropdown view:
List view:
Fixes #1464.