Skip to content

Widget Visibility: Rewrite the way that the major/minor rules lists are generated.#3444

Closed
cfinke wants to merge 2 commits intoAutomattic:masterfrom
cfinke:master
Closed

Widget Visibility: Rewrite the way that the major/minor rules lists are generated.#3444
cfinke wants to merge 2 commits intoAutomattic:masterfrom
cfinke:master

Conversation

@cfinke
Copy link
Copy Markdown
Contributor

@cfinke cfinke commented Feb 23, 2016

Widget Visibility: Rewrite the way that the major/minor rules lists are generated to save bandwidth and memory.

In situations where a site has hundreds of widgets each with multiple visibility rules, the widget admin pagesize can grow out of control, causing out-of-memory errors and issues with loading the customizer.

This commit rewrites the way that the admin UI is generated, moving from hardcoding everything server-side to providing a single set of all the possible visibility options that are then dynamically generated client-side when they're needed.

This also has the happy side-effect of eliminating the need for the AJAX calls when changing the major rule type. It also fixes some inconsistencies with how the "Include children" checkbox was processed -- in some circumstances, the "Include children" option was being applied to the wrong set of conditions.

…re generated to save bandwidth and memory.

In situations where a site has hundreds of widgets each with multiple visibility rules, the widget admin pagesize can grow out of control, causing out-of-memory errors and issues with loading the customizer.

This commit rewrites the way that the admin UI is generated, moving from hardcoding everything server-side to providing a single set of all the possible visibility options that are then dynamically generated client-side when they're needed.

This also has the happy side-effect of eliminating the need for the AJAX calls when changing the major rule type. It also fixes some inconsistencies with how the "Include children" checkbox was processed.
@kraftbj kraftbj added this to the 4.0 milestone Feb 23, 2016
@jeherve jeherve added the Enhancement Changes to an existing feature — removing, adding, or changing parts of it label Feb 24, 2016
@dereksmart
Copy link
Copy Markdown
Contributor

Failing some jshint stuff, can you take a look? https://travis-ci.org/Automattic/jetpack/jobs/111335805

Address some comparison and variable declaration issues, mostly by eliminating them alltogether by using a simpler method of setting the value of a select input.
@jeherve jeherve 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] Needs Review This PR is ready for review. labels Jun 17, 2016
@jeherve jeherve modified the milestones: 4.2, 4.1 Jun 17, 2016
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Jun 17, 2016

Sorry we didn't get around to it until now. It looks like it will need a rebase now.

@jeherve jeherve modified the milestones: 4.3, 4.2 Jul 6, 2016
@richardmuscat richardmuscat modified the milestones: 4.3, 4.4 Jul 7, 2016
@jeherve jeherve modified the milestone: 4.4 Jul 8, 2016
@samhotchkiss samhotchkiss modified the milestones: Not Currently Planned, 4.4 Nov 9, 2016
@jeherve
Copy link
Copy Markdown
Member

jeherve commented Nov 30, 2016

#5781

@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Feb 8, 2017

Rebase in progress...

@zinigor
Copy link
Copy Markdown
Contributor

zinigor commented Feb 8, 2017

Closing in favour of #6333. Hopefully it won't get closed in favour of 9222.

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 [Focus] Performance [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] Tested on WP.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants