WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 6 days ago

#15800 new enhancement

Adding tabs to the "Edit Site"-pages in Network Admin

Reported by: PuffyThePirateBoy Owned by:
Milestone: 4.6 Priority: normal
Severity: minor Version: 3.1
Component: Networks and Sites Keywords: has-patch needs-testing
Focuses: accessibility, administration, multisite Cc:

Description

There are four tabs in the Network Admin > Edit Site pages. These are statically defined as an array in the files:

wp-admin/network/site-info.php
wp-admin/network/site-options.php
wp-admin/network/site-themes.php
wp-admin/network/site-users.php

It would be nice if there was a filter that allowed us to add more tabs to this bar. This would enable us to add more user friendly, site specific, option pages for super administrators.

Attachments (9)

15800.patch (6.1 KB) - added by c3mdigital 2 years ago.
tab-tester.php (1.5 KB) - added by c3mdigital 2 years ago.
Test case plugin add new tab plugin
0001-Enhance-tabs-15800.patch (7.3 KB) - added by Bueltge 2 years ago.
network-tab-tester-15800.php (1.5 KB) - added by Bueltge 19 months ago.
Tab tester, readable simple source
15800.png (21.1 KB) - added by Bueltge 19 months ago.
Screenshot of tab tester example plugin
Screen Shot 2016-03-06 at 1.09.34 PM.png (72.7 KB) - added by johnjamesjacoby 2 months ago.
Bigger tabs
15800.2.patch (10.0 KB) - added by johnjamesjacoby 2 months ago.
Refresh, and refactor the original patch a bit
15800.3.patch (10.2 KB) - added by johnjamesjacoby 8 weeks ago.
Add selected argument, and pass from each core file. This makes it easier for custom pages to target and render what tab is "active" without needing to guess at globals or callback hooks.
Screen Shot 2016-04-18 at 3.15.12 PM.png (54.6 KB) - added by johnjamesjacoby 2 weeks ago.
Tabs removed; Tab added

Download all attachments as: .zip

Change History (44)

#1 @ocean90
5 years ago

  • Version set to 3.1

See #15593. There was already a patch from PeteMall.

#2 @westi
5 years ago

This is a great idea for the future.

#3 @westi
5 years ago

  • Milestone changed from Awaiting Review to Future Release

#4 @PuffyThePirateBoy
5 years ago

In #15593 this enhancement was requested as part of an other issue (missing previously existing hooks). However, the patch only applies to the other issue leaving the enhancement request overlooked.

#5 @bananastalktome
4 years ago

  • Cc bananastalktome@… added

+1 on this request. It would be great to be able to add additional tabs to the page. Any chance of this being revisited (or guidance for others who may want to submit a patch for this)?

#6 @cfoellmann
3 years ago

I could really use such a hook.
Maybe I have some time to look into it soon.

#7 @cfoellmann
3 years ago

  • Cc foellmann@… added

#8 @gogger
3 years ago

  • Cc gogger added

#9 @Bueltge
2 years ago

  • Cc frank@… added

#10 @toscho
2 years ago

  • Cc info@… added

#11 @cfoellmann
2 years ago

Any idea for a starting point? Crude idea how we should tackle this?

#12 @cfoellmann
2 years ago

Whoever wants to pitch in have a look at this repo https://github.com/cfoellmann/WordPress-network-tabs - I will give you push access if you reply here: https://github.com/cfoellmann/WordPress-network-tabs/issues/1

#13 @nacin
2 years ago

  • Component changed from Network Admin to Networks and Sites
  • Focuses administration added

#14 @c3mdigital
2 years ago

Found this ticket working on extending ourStream plugin to work with multisite. We need to be able to add a settings page for each site on the network so that when network activated individual site settings can be updated via the network admin menu.

I think we should stick with the scope of this ticket and while it would be nice to have a generic way to add tabs to any page my patch focuses on extending the Edit- site pages tabs and consolidating the repeated hard coded tabs html into a function that can be filtered to add the extra tab. I've also added a test case plugin that tests adding a new tab. The patch needs to be refined as it's more of a test of concept. Please provide feedback.

@c3mdigital
2 years ago

@c3mdigital
2 years ago

Test case plugin add new tab plugin

#15 @rhurling
2 years ago

  • Keywords has-patch dev-feedback added

#16 @Bueltge
2 years ago

  • Focuses accessibility added

I have enhance the patch from @c3mdigital for a little bit codex and advanced ARIA.

See also on this commit https://github.com/cfoellmann/WordPress/commit/141f5b8944355c9c544da3bae7feefdaf2f461b5

Last edited 2 years ago by Bueltge (previous) (diff)

#17 @cfoellmann
2 years ago

Usage example by @bueltge https://github.com/cfoellmann/WordPress/issues/3#issuecomment-41380678

I think we have a working solution here. I would really love the Settings API to support tabs but that is asking too much at the moment.

Last edited 2 years ago by cfoellmann (previous) (diff)

@Bueltge
19 months ago

Tab tester, readable simple source

@Bueltge
19 months ago

Screenshot of tab tester example plugin

#18 @Bueltge
19 months ago

After discussion on contributor day I have created a new, simple example plugin to explain how it works - network-tab-tester-15800.php.

The plugin shows a basic example creating a tab named test, slug test_tab and creates a callback ticket15800_callback that only runs when the tab is active to get the content.

Screenshot of tab tester example plugin

Last edited 19 months ago by Bueltge (previous) (diff)

#19 @basteln3rk
10 months ago

Would be very useful to have this merged as the current workarounds, such as using jQuery to inject custom tabs through admin footer, see http://wordpress.stackexchange.com/questions/103765/add-site-options-ui-in-multisite-sites-infos-page, are not nice and several plugins could use this.

This ticket was mentioned in Slack in #core-multisite by bueltge. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-multisite by bueltge. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


9 months ago

#23 @afercia
8 months ago

From an accessibility point of view, these "tabs" are not real tabs, not as an ARIA tabbed interface is intended, see http://www.w3.org/TR/wai-aria-practices/#tabpanel and the examples linked there. They just look like tabs but actually they're just links to other pages. Thus, they shouldn't use any ARIA roles or properties.
We discussed this a bit in the accessibility team in a ticket related to the "tabs" in the menus screen, see #33603 and considering to just wrap them in a navigation landmark.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


2 months ago

#25 @johnjamesjacoby
2 months ago

  • Keywords early added

15800.2.patch does the following:

  • Refreshes patch for WordPress 4.6-early
  • Adds a cap to each tab, so they can be conditionally checked and/or skipped
  • Modifies each of the 4 files referenced in core tabs to include their new capabilities: (manage_site_info, manage_site_themes, manage_site_users, manage_site_settings)
  • Adds more sane filters on tabs and arguments vs. HTML output
  • Removes the nav-tab-small classification, because big is beautiful and this is the last remaining/only place in core that uses these tiny tabs
  • Moves the wrapper HTML into before and after arguments that can be filtered, should someone want to get fancy with this interface on their own without modifying core HTML
  • Adds some output buffering to encapsulate the final output of all of this processing and calculation
  • I decided not to include the callback idea previously explored in this ticket. I think it's a good idea, but out of scope for the general improvement of encapsulating these tabs into a central DRY approach. Once we get that done, we should probably make new tickets for future enhancements.

A few talking points after chatting with @jeremyfelt:

  • What if we want to abandon these tabs later? Personally, I don't think we should, because these tabs make for a convenient and useful interface for power users in a trusted position. I can see making this fancy like the Appearance bar, but removing them seems drastic.
  • How do we want to really truly allow these tabs to be filtered? I can imagine a bunch of ways, but I used Mercator as an example of adding domain mapping here. Right now, it has to use JavaScript to inject it's tab at the end, which isn't very great. I believe tabs alone could be filtered, and that someone might want to totally override this interface to suit their power-user needs, so I've included 2 filters for both of those purposes.
  • Did we get the ARIA tags right? I think there's much more we can do here, but what's in the patch seems like the bare minimum and follows what WordPress is already trying to do.
  • Is the output buffer absolutely necessary? No, but I like the idea of batching this output and rendering it all at once. Why do this? In the unlikely event of OOM errors, no tabs render vs partial or incomplete tabs. It also prevents jumpiness on slow connections while tabs are looped through and echo'ed. I have a tendency to do this with most loops that shouldn't communicate to the user if an interruption occurs, so I included that philosophy here. WordPress core also does this in several places already, though I consider it an unwritten rule.
  • I have a somewhat selfishly urgent need to see this happen in 4.6-early, so whatever I can do to get yay/nay from interested parties and a sign-off from Jeremy, I will keep an eye on this and shepherd as needed.
  • Lastly, is the function name network_edit_site_tabs() acceptable to everyone? All possible function names I can think of for this function are equally bad. wp_network_site_edit_tabs() wp_network_edit_site_tabs() maybe_wpms_site_edit_tabs_are_too_big() et all
Last edited 2 months ago by johnjamesjacoby (previous) (diff)

@johnjamesjacoby
2 months ago

Refresh, and refactor the original patch a bit

@johnjamesjacoby
8 weeks ago

Add selected argument, and pass from each core file. This makes it easier for custom pages to target and render what tab is "active" without needing to guess at globals or callback hooks.

This ticket was mentioned in Slack in #core-multisite by jjj. View the logs.


3 weeks ago

#27 @jeremyfelt
3 weeks ago

  • Keywords needs-testing added; dev-feedback removed

I've only had time to give this a read-through, but a general +1 on where things are going. :)

A couple thoughts:

  • Adding more specific capability checks makes sense. I'd probably prefer to introduce that granularity in another ticket as it will involve accounting for the current manage_sites capability in a back-compat way.
  • I do wonder if network_edit_site_tabs is the right place to loop through the capabilities. Some of that could instead be handled by plugins as they choose whether or not to add a tab to the filter.
  • What would it look like to exclude the core provided tabs from the network_edit_site_tabs filter? Once more granular capabilities are available, tabs could be hidden on a cap check. This would align more with the result of visiting each of these pages directly.
  • I'm not sure about the before/after HTML args for the tab output.

#28 @jeremyfelt
3 weeks ago

  • Keywords early removed
  • Milestone changed from Future Release to 4.6

Bumping this into the 4.6 milestone, because the time is appropriate to figure this out. Feedback is very welcome. :)

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


2 weeks ago

#30 @afercia
2 weeks ago

Discussed a bit in today's a11y bugs scrub meeting. From an accessibility and semantic point of view, these are not "tabs". They're just links that look like tabs. More like a navigation sub-menu. ARIA tablist, tab, tabpanel roles are intended for "in page" content that gets revealed / hidden when clicking on a tab. Simple example here:
http://heydonworks.com/practical_aria_examples/#tab-interface

We'd recommend to remove any ARIA added here. Also in the accessibility team we're planning to review all the different kind of "tabs" used in the admin, hopefully in an incoming feature (plugin) project.

#31 @johnjamesjacoby
2 weeks ago

Responding to @jeremyfelt's points:

  • I'm fine doing that elsewhere, but we'll need to be doubly sure we don't make any mistakes during the fork and merge of these two initiatives
  • The way we've been doing this everywhere else is to make these screens objects, but we can't easily have more than 1 WP_Screen object hanging around at any one time. I think because these tabs are put out at run time vs. precompiled on init or elsewhere, doing this late is the only place we currently have without inventing a new place
  • It looks fine until you start to have no tabs, which indicates something has gone horribly wrong. We should account for this, but it would require manually unsetting something in a forceful way, which makes me inclined to think someone doing that wants that level of control.
  • I know we don't do this in very many places anymore, but considering this is duplicated logic across 4 (or more) files, it felt right to put it inside the function. Once it was in there, I could imagine core wanting to modify this in the future, and/or plugins wanting to go even further into modifying the tabbed interface into something else. Naturally, I'm doing this, so I find it pretty useful as it is.
Last edited 2 weeks ago by johnjamesjacoby (previous) (diff)

@johnjamesjacoby
2 weeks ago

Tabs removed; Tab added

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


11 days ago

This ticket was mentioned in Slack in #core-multisite by richardtape. View the logs.


10 days ago

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


6 days ago

#35 @iamfriendly
6 days ago

Just a couple of things to note (and it's me being intentionally destructive in this testing). Apologies if this is kinda obvious.

  1. If you unset 'site-info' from the $tabs array in the network_edit_site_tabs filter, you still see the fields when visiting wp-admin/network/site-info.php?id=<id> . The same goes if you empty the array of tabs, naturally.

This might be just a 'well, cool, you can break stuff if you really want to' kinda scenario. I guess my point is it might be strange to someone to remove a tab from the list and its fields still appear.

  1. Removing the ARIA attributes from the patch doesn't appear to have any negative issues as far as I can tell. It's still keyboard accessible, no visual differences, nothing weird in Safari voiceover.
Note: See TracTickets for help on using tickets.