-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix: Improve consistency of the Dashboard widgets when no-js #12677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
70ce162
9d8a50c
f9da145
dd36c5b
a66d8af
8fcf993
52208ec
8d27ed0
f1432b9
cae909f
2dbb057
25c01b2
27aa72d
1466133
b94a890
92ddb29
3590029
5522b0d
8b8d8b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -656,6 +656,9 @@ function wp_dashboard_recent_drafts( $drafts = false ) { | |
|
|
||
| $drafts = get_posts( $query_args ); | ||
| if ( ! $drafts ) { | ||
| echo '<div class="no-drafts">'; | ||
| echo '<p>' . __( 'There are no recent drafts.' ) . '</p>'; | ||
| echo '</div>'; | ||
| return; | ||
| } | ||
| } | ||
|
|
@@ -1372,7 +1375,7 @@ function wp_dashboard_events_news() { | |
| * @since 4.8.0 | ||
| */ | ||
| function wp_print_community_events_markup() { | ||
| $community_events_notice = '<p class="hide-if-js">' . __( 'This widget requires JavaScript.' ) . '</p>'; | ||
| $community_events_notice = '<p class="hide-if-js">' . __( 'The WordPress Events and News feeds require JavaScript.' ) . '</p>'; | ||
| $community_events_notice .= '<p class="community-events-error-occurred" aria-hidden="true">' . __( 'An error occurred. Please try again.' ) . '</p>'; | ||
| $community_events_notice .= '<p class="community-events-could-not-locate" aria-hidden="true"></p>'; | ||
|
hbhalodia marked this conversation as resolved.
|
||
|
|
||
|
|
@@ -1998,7 +2001,7 @@ function wp_dashboard_site_health() { | |
| $issues_total = $issue_counts['recommended'] + $issue_counts['critical']; | ||
| ?> | ||
| <div class="health-check-widget"> | ||
| <div class="health-check-widget-title-section site-health-progress-wrapper loading hide-if-no-js"> | ||
| <div class="health-check-widget-title-section site-health-progress-wrapper loading"> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this makes sense to me, Because users won't be shown anything even if they visit the page showed in the message, so that's good idea to not display the widget entirely when there is no-js. Though there is no any direct way, but we may need to use filter add add the diff --git a/src/wp-admin/includes/dashboard.php b/src/wp-admin/includes/dashboard.php
index daa6913a92..0b193eae74 100644
--- a/src/wp-admin/includes/dashboard.php
+++ b/src/wp-admin/includes/dashboard.php
@@ -65,6 +65,9 @@ function wp_dashboard_setup() {
wp_enqueue_style( 'site-health' );
wp_enqueue_script( 'site-health' );
+ // The widget's data is loaded via JavaScript, so hide the whole widget when JavaScript is disabled.
+ add_filter( 'postbox_classes_dashboard_dashboard_site_health', 'wp_dashboard_site_health_postbox_class' );
+
wp_add_dashboard_widget( 'dashboard_site_health', __( 'Site Health Status' ), 'wp_dashboard_site_health' );
}
@@ -1820,6 +1823,23 @@ function dashboard_browser_nag_class( $classes ) {
return $classes;
}
+/**
+ * Adds a class to the Site Health dashboard widget's meta box.
+ *
+ * The widget's data is loaded via JavaScript, so the whole widget is hidden
+ * when JavaScript is disabled.
+ *
+ * @since 7.1.0
+ *
+ * @param string[] $classes Array of meta box classes.
+ * @return string[] Modified array of meta box classes.
+ */
+function wp_dashboard_site_health_postbox_class( $classes ) {
+ $classes[] = 'hide-if-no-js';
+
+ return $classes;
+}
+
/**
* Checks if the user needs a browser update.
*I am updating the PR for now, we can revert the change if not needed. Thanks,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The filter is unnecessary. Reminder that when JS is off, the body element has a CSS class
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I have update to use CSS instead of adding the filter. |
||
| <div class="site-health-progress"> | ||
| <svg aria-hidden="true" focusable="false" width="100%" height="100%" viewBox="0 0 200 200" version="1.1" xmlns="http://www.w3.org/2000/svg"> | ||
| <circle r="90" cx="100" cy="100" fill="transparent" stroke-dasharray="565.48" stroke-dashoffset="0"></circle> | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I like this message and I'd consider to show it also when JS is on.
It shouldn't be a H2 though. Headings should only be used to identify a section of content and htere's no content here. A paragraph would be best.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @afercia, Thanks for the review,
When JS is on, we are already showing the input form and when there are drafts that are already been show as expected. If there are no drafts we are not showing anything as we have input form which works as expected. Do you want to add extra messaging? but I see there is not point in terms of UX as it is since long time and users did'nt notice that.
I would review this as we may need to update some styles here when using
paragraph. Will update the PR with the same.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: Have fixed to use
<p>tag and update a style same as<h2>which was used to show the below message where there are draft and js is enabled.Thanks,
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know. Personally, I think the 'no drafts' message is useful and it should be shown anyways:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @afercia, I have implemented the suggestion, as it would only be better for users to get a proper messaging.
See the commit - 92ddb29