Issue 484 - Combine Connectors and Contexts into a single column #537
Conversation
…he context dropdown
Conflicts: includes/connectors.php
Conflicts: includes/list-table.php
| @@ -160,6 +157,14 @@ function get_records() { | |||
| } | |||
| $args['paged'] = $this->get_pagenum(); | |||
|
|
|||
| $context = explode( '-', wp_stream_filter_input( INPUT_GET, 'context' ) ); | |||
There was a problem hiding this comment.
@lukecarbis This logic seems to fail when you have a context that is multiple words, such as Main Menu or Primary Sidebar.
There was a problem hiding this comment.
From memory I thought this would work because these are always underscored, not dashed.
There was a problem hiding this comment.
No, the context for Main Menu shows up as context=menus-main-menu.
Which executes a query for $args['connector'] = 'menus' and $args['context'] = 'main'.
There was a problem hiding this comment.
@lukecarbis My thought is the cleanest way to implement this would be to use JS to draw a distinction between top-level items and sub-level items in the Context input and use the proper query string based on that.
e.g. ?connector=menus&context=main-menu
|
Thanks for your help @fjarrett - this is looking good. Please continue your review. 😺 |
| @@ -756,3 +788,9 @@ jQuery(function($){ | |||
| intervals.init( $('.date-interval') ); | |||
| }); | |||
| }); | |||
|
|
|||
| jQuery.extend({ | |||
| getQueryParameters : function( str ) { | |||
There was a problem hiding this comment.
@lukecarbis Do you think we should prefix this function name?
Copy pasta straight from: http://css-tricks.com/snippets/jquery/get-query-params-object/
There was a problem hiding this comment.
Yes! Good pickup.
|
@lukecarbis Oh, it's also worth noting that Data Exporter, Notifications and Reports will all need to be updated to use this new UI. |
Conflicts: ui/admin.js
|
@lukecarbis This looks great. I'm going to close this PR though and please reissue one against the |
Resolves #484.