Opened 9 years ago
Closed 8 years ago
#35021 closed defect (bug) (fixed)
Dashboard widgets doubled "Configure" link
Reported by: | afercia | Owned by: | nicholas_io |
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Widgets | Keywords: | has-screenshots good-first-bug dev-feedback has-patch |
Focuses: | ui, accessibility | Cc: |
Description
Noticed while investigating on #34987: the Dashboard widgets with a "Configure" link have two "Configure" links. The first one is inside the toggle button and even if it's visually hidden, it's still focusable and can be activated using a keyboard. The second one is inside the heading.
This is because the link HTML gets appended to the Widget name, see wp_add_dashboard_widget()
and then the widget name is passed to add_meta_box()
then in a global and finally used by do_meta_boxes()
, both inside the toggle button and the heading. (phew!)
To check this you'd need to install a Plugin that provides a configurable Dashboard widget.
For now, in #34987 the first link gets hidden with CSS. A better solution would be avoiding to output useless markup in the first place.
Ideally, the widget name should be just a string with no HTML and the link output should use a separate variable but then a new argument would be required. To keep it simple maybe the HTML part of the string could be removed just before the output with explode()
or something like that. A bit weak though. Open to suggestions :)
Attachments (3)
Change History (15)
#2
@
9 years ago
Adding a new param like button_title
is straightforward but it would depend heavily on developers using it, which is unlikely to happen since there's no visual change, "just" an accessibility improvement (unfortunately accessibility doesn't matter for most developers).
Another idea is to use $callback_args
to add an internal/private param like __widget_basename
in wp_add_dashboard_widget()
and pass this param to the do_meta_boxes()
function and check there if there's a $callback param called _widget_basename
and use that value instead of the $widget_name
and then unset this internal param do avoid passing it to the user callback function.
I've just attached a patch to implement this idea.
#5
@
9 years ago
- Milestone changed from Awaiting Review to Future Release
Thanks for the patch, @nicholas_io. That's a neat approach!
In 35021.diff I extended it to fix the same issue in the screen options as reported in #36158.
#6
@
9 years ago
- Milestone changed from Future Release to 4.6
Would be great if we could fix this in 4.6
#7
@
8 years ago
- Owner set to nicholas_io
- Status changed from new to assigned
Assigning to mark the good-first-bug as "claimed".
#9
@
8 years ago
- Keywords needs-patch added; has-patch removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening because of #37298
PHP Fatal error: Cannot use object of type WP_Post_Type as array in .../wp-admin/includes/screen.php on line 123
I've been playing with using
wp_strip_all_tags()
indo_meta_boxes
for a bit.It doesn't work too bad, but there are widgets that to some strange strings. The Quick Draft meta box for example has the following title:
<span class="hide-if-no-js">Quick Draft</span> <span class="hide-if-js">Drafts</span>
It would be more safe to allow people passing a separate title that can be used for that button to
wp_add_dashboard_widget()
. Example:wp_add_dashboard_widget( 'dashboard_quick_press', $quick_draft_title, 'wp_dashboard_quick_press', null, array( 'button_title' => 'test', ) );
The data in the
$callback_args
array can be checked for the existence of this title. If it doesn't exist, the widget title can be used as usual.Just needs a better name instead of
button_title
, some docs and perhaps a review of what else this could be used for.