WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 9 days ago

#43697 reviewing enhancement

Add theme update counter in admin theme menu item

Reported by: mukesh27 Owned by: SergeyBiryukov
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Themes Keywords: has-patch has-screenshots dev-feedback needs-testing has-testing-info
Focuses: ui, administration Cc:

Description

Add theme update counter in admin theme menu item as we show for plugin menu item.

Attachments (7)

theme-update.png (617.9 KB) - added by mukesh27 3 years ago.
43697.patch (1.3 KB) - added by mukesh27 3 years ago.
43697.2.patch (1.3 KB) - added by mukesh27 3 years ago.
updated patch
43697.3.diff (1.4 KB) - added by zodiac1978 3 months ago.
Updated patch. Count is added to "appearance" now and is using the correct class names and caps.
Bildschirmfoto 2021-02-06 um 22.07.04.png (45.5 KB) - added by zodiac1978 3 months ago.
New screenshot showing the badge
43697.4.diff (1.4 KB) - added by zodiac1978 3 months ago.
Corrected wrong comment text.
Screen Shot 2021-05-07 at 14.53.33.png (460.5 KB) - added by francina 9 days ago.
Notification appears and matches where there is an update notification

Download all attachments as: .zip

Change History (25)

@mukesh27
3 years ago

@mukesh27
3 years ago

#1 @mukesh27
3 years ago

  • Keywords has-patch has-screenshots added

#2 @joyously
3 years ago

Why does your code use the update-plugins capability and class name?

This used to work. Can you look back at older versions for that code?

Last edited 3 years ago by joyously (previous) (diff)

@mukesh27
3 years ago

updated patch

#3 @mukesh27
3 years ago

ohhh, i have updated new patch

#4 @pento
2 years ago

  • Version trunk deleted

#5 @SergeyBiryukov
3 months ago

#52425 was marked as a duplicate.

#6 @zodiac1978
3 months ago

  • Focuses ui added

@joyously I think using update-plugins as class name is not very intuitive, but at the moment correct.

The network menu is using it too:
https://github.com/WordPress/WordPress/blob/b3b8942dfcb451eddb5559b63c1043fce5d9449e/wp-admin/network/menu.php#L65

We could either use it here too or add the class name to the CSS, because at the moment it does not exist:
https://github.com/WordPress/WordPress/search?q=update-themes

@mukesh27 Looking at the patch and the screenshot it shows an inconsistent behavior: The badge is added to the first submenu item instead of the main menu item ("Appearance"), like it is seen for the "Plugins" menu item.

I would vote for using the same design as for Plugins (which is also used in a multisite):
https://github.com/WordPress/WordPress/blob/b3b8942dfcb451eddb5559b63c1043fce5d9449e/wp-admin/network/menu.php#L59-L79

@zodiac1978
3 months ago

Updated patch. Count is added to "appearance" now and is using the correct class names and caps.

@zodiac1978
3 months ago

New screenshot showing the badge

#7 @zodiac1978
3 months ago

I am still experiencing a strange behavior. There is a paid theme in my test environment with available update, but the notice "Automatic update is unavailable for this theme."

After leaving the themes.php page the count is changing from 7 to 6. On all admin pages it stays on 6. Just on update-core.php it is showing 7 again.

#8 @zodiac1978
3 months ago

  • Keywords dev-feedback needs-testing added

@zodiac1978
3 months ago

Corrected wrong comment text.

#9 @SergeyBiryukov
3 months ago

  • Milestone changed from Awaiting Review to 5.8
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#10 @Boniu91
2 weeks ago

  • Keywords has-testing-info added

This ticket was mentioned in Slack in #core-test by boniu91. View the logs.


9 days ago

#12 @Boniu91
9 days ago

What to test:

  1. If it displays the notification correctly
  2. If it's removed after the update
  3. Check it on the PHP8
  4. Compare the number with the number inside the Dashboard > Updates
  5. Check the multisite
  6. Check the display on mobiles
  7. Confirm that it works with Paid Themes

This ticket was mentioned in Slack in #core-test by dariak. View the logs.


9 days ago

#14 @francina
9 days ago

Tested

  • MacOS 11.3.1
  • Safari 14.1
  • WordPress 5.8-alpha-50427-src
  1. Rolled back GeneratePress free and Twenty Twenty-one to previous versions
  2. Applied the patch
  3. The notification now appears on the Appearance menu
  4. The number of updates matches in:
  • Dashboard > Updates
  • Appearance
  • Horizontal bar
  1. The notification disappears once I update the themes
Last edited 9 days ago by francina (previous) (diff)

@francina
9 days ago

Notification appears and matches where there is an update notification

This ticket was mentioned in Slack in #core-test by dariak. View the logs.


9 days ago

#16 follow-up: @sannevndrmeulen
9 days ago

Test feedback:

  • MacOS 10.14.6
  • Google Chrome 90.0.4430.93
  • WordPress 5.8-alpha-50821
  1. Once I got the patch working I see that the notification is displayed behind Appearance.
  2. When I update a theme the notification updates too, lowering the number of remaining themes that require an update.
  3. -
  4. The number behind Appearance matches the number shown for Dashboard -> Updates.
  5. -
  6. Same behaviour as in step 1, 2, and 4. However, I did notice that the placement of the notification bubbles looks slightly different on mobile.
  7. -

Steps that still require testing:

  • Check it on the PHP8
  • Check the multisite
  • Confirm that it works with Paid Themes

#17 in reply to: ↑ 16 @zodiac1978
9 days ago

Replying to sannevndrmeulen:

Steps that still require testing:

  • Check it on the PHP8
  • Check the multisite
  • Confirm that it works with Paid Themes

Thanks at you all for testing the patch! @francina @sannevndrmeulen

I will try to check this with PHP 8. But as this code is mainly adopted from the network version this should work fine.

Multisite is already showing this badge, because it uses another code. This code just runs on single sites.

The main reason for the needs-testing tag was my experience with paid themes (see comment above).

It would be great if someone can try to test this patch with a paid theme which needs an update and tries to reproduce the described problem.

#18 @zodiac1978
9 days ago

Check on PHP 8 is positive. Works without problems.

Updates on theme page, theme overview and update core page are working and the count is changed accordingly after update.

Note: See TracTickets for help on using tickets.