Skip to content

Widget Visibility: support custom post type archive pages #80#2982

Merged
samhotchkiss merged 4 commits intomasterfrom
fix/widget-visibility
Apr 4, 2016
Merged

Widget Visibility: support custom post type archive pages #80#2982
samhotchkiss merged 4 commits intomasterfrom
fix/widget-visibility

Conversation

@jessefriedman
Copy link
Copy Markdown
Member

fixes for #80 and #750
replaces #1599

@jessefriedman jessefriedman self-assigned this Nov 9, 2015
@jeherve jeherve added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Widget Visibility [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Nov 10, 2015
@jeherve jeherve added this to the 3.9 milestone Nov 10, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This string "Show/Hide if user in certain custom post type archive." should be "Show/Hide if user is in a certain custom post type archive."

@eliorivero
Copy link
Copy Markdown
Contributor

Works good. Needs update to solve conflicts.

@eliorivero eliorivero added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Jan 14, 2016
@jeherve jeherve modified the milestones: 4.0, 3.9 Jan 14, 2016
@jeherve jeherve modified the milestones: 3.9.2, 4.0 Feb 12, 2016
@jeherve jeherve force-pushed the fix/widget-visibility branch from da2d9d6 to 06c8cef Compare February 17, 2016 15:03
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Feb 17, 2016

Rebased.

<option value="date" <?php selected( "date", $rule['major'] ); ?>><?php echo esc_html_x( 'Date', 'Noun, as in: "This page is a date archive."', 'jetpack' ); ?></option>
<option value="page" <?php selected( "page", $rule['major'] ); ?>><?php echo esc_html_x( 'Page', 'Example: The user is looking at a page, not a post.', 'jetpack' ); ?></option>

<option value="post_type" <?php selected( "post_type", $rule['major'] ); ?>><?php echo esc_html_x( 'Post Type', 'Show/Hide if user is in a certain custom post type archive.', 'jetpack' ); ?></option>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a very strange Translator Context comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed. Do we really need a comment there? "Post Type" seems pretty standard.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree. I've modified it following the wording of the one used for Page

@dereksmart dereksmart modified the milestones: 3.9.2, 3.9.3 Feb 24, 2016
@eliorivero eliorivero removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Feb 26, 2016
@zinigor zinigor modified the milestones: 3.10, 3.9.3 Mar 3, 2016
@zinigor zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 31, 2016
@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Mar 31, 2016

Tested, works well. I have adjusted the text, as well as refactored the markup a bit.

@samhotchkiss samhotchkiss merged commit d7e974e into master Apr 4, 2016
@samhotchkiss samhotchkiss deleted the fix/widget-visibility branch April 4, 2016 00:35
@samhotchkiss samhotchkiss removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Widget Visibility Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants