Skip to content

Widget Visibility: add ability to match ALL conditions.#3289

Closed
atrus1701 wants to merge 1 commit intoAutomattic:masterfrom
atrus1701:widget-visibility-852
Closed

Widget Visibility: add ability to match ALL conditions.#3289
atrus1701 wants to merge 1 commit intoAutomattic:masterfrom
atrus1701:widget-visibility-852

Conversation

@atrus1701
Copy link
Copy Markdown
Contributor

This fixes issue #852.

  • Add the option to match all conditions specified for the widget.

This is related to Pull Request #3210. This original Pull Request contained several fixes that are being broken out into several smaller requests.

- Add the option to match all conditions specified for the widget.
- see Automattic#852.
@jeherve jeherve added [Feature] Widget Visibility Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. labels Jan 18, 2016
@jeherve jeherve added this to the 3.9.1 milestone Jan 18, 2016
@kraftbj kraftbj modified the milestones: 3.9.1, 3.9.2 Jan 21, 2016
@dereksmart dereksmart modified the milestones: 3.9.2, 3.9.3 Feb 24, 2016
@zinigor zinigor modified the milestones: 3.10, 3.9.3 Mar 3, 2016
@samhotchkiss samhotchkiss modified the milestones: 3.10, Widget Visibility Next Mar 28, 2016
@jeherve jeherve modified the milestones: 4.2, Widget Visibility Next Jun 17, 2016
@jeherve jeherve modified the milestones: 4.3, 4.2 Jul 6, 2016
@richardmuscat richardmuscat modified the milestones: 4.3, 4.4 Jul 7, 2016
@richardmuscat richardmuscat removed this from the 4.3 milestone Jul 7, 2016
<span class="condition-conjunction"><?php echo esc_html_x( 'or', 'Shown between widget visibility conditions.', 'jetpack' ); ?></span>
<span class="condition-conjunction">
<?php
// echo esc_html_x( 'or', 'Shown between widget visibility conditions.', 'jetpack' );
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.

Ideally, we would use a bit of Javascript to alternate this value between "or" and "and" depending on the state of the checkbox that was introduced.

If we are going to omit this label instead, we should remove this line rather than comment out the text.

@rcoll
Copy link
Copy Markdown
Member

rcoll commented Aug 26, 2016

From a functionality standpoint, this looks good to me and seems to work well in each case that I tested. However, I'd like to see a bit of input on how the "Match All" checkbox is positioned. I also left an inline comment about using a bit of Javascript to alter the "and" and "or" values of the conjunction label.

Perhaps a second set of experienced eyes on this and/or a quick design review would be good.

@rcoll rcoll added [Status] Ready to Merge Go ahead, you can push that green button! [Status] Needs Design Review Design has been added. Needs a review! and removed [Status] Needs Review This PR is ready for review. labels Aug 26, 2016
@samhotchkiss samhotchkiss added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Nov 8, 2016
@samhotchkiss
Copy link
Copy Markdown
Contributor

Bumping this to 4.5 while we wait for @atrus1701 to reply-- @atrus1701, if you can address Rich's concerns, we'd love to get this into 4.5 which will be released next month.

Thanks!

@samhotchkiss samhotchkiss modified the milestones: 4.5, 4.4 Nov 8, 2016
@MichaelArestad
Copy link
Copy Markdown
Contributor

Can you add a quick screenshot?

@jeherve jeherve modified the milestones: Widget Visibility Next, 4.5, Not Currently Planned Nov 30, 2016
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Nov 30, 2016

#5781

@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Feb 8, 2017

Closing in favor of #6327

@zinigor zinigor closed this Feb 8, 2017
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 [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Needs Design Review Design has been added. Needs a review! Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants