Make WordPress Core

Opened 10 years ago

Closed 10 years ago

#31739 closed defect (bug) (fixed)

Shiny Updates - support for details modal window

Reported by: pavelevap's profile pavelevap Owned by: jorbin's profile jorbin
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.2
Component: Upgrade/Install Keywords: has-patch
Focuses: Cc:

Description

When updating plugins, I usually click "View details" before updating. But when I click "Update" from opened modal window, Shiny Updates are not used, but I am redirected from Plugins administration page. It would be nice to preserve current page, close modal window and run Shiny Updates also in this case.

Attachments (1)

31739.diff (2.2 KB) - added by jorbin 10 years ago.

Download all attachments as: .zip

Change History (18)

#1 @jorbin
10 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.2

Thanks Pavelevap

For this, we are going to need to bind an event to the upgrade click that:

1) closes the window
2) Initiates the appropriate update.

The one thing I'm not sure of is where we should put focus when this happens.

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


10 years ago

#3 @jorbin
10 years ago

  • Owner set to jorbin
  • Status changed from new to assigned

#4 @joedolson
10 years ago

I'd think focus should be set to where the progress messages are shown. Does that seem logical?

#5 @afercia
10 years ago

About focus, see similar issue faced with FTP credentials modal

@jorbin
10 years ago

#6 @jorbin
10 years ago

  • Keywords has-patch added; needs-patch removed

The above patch adds support for shiny updates from the details modal window to browsers that support postMessage. Since it's an iFrame, this is the easiest method and I think we are ok not supporting below IE8 in this instance.

I set focus on the update message box. Feedback is welcome there.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


10 years ago

#8 @afercia
10 years ago

@jorbin hi,
if I'm not wrong, the "update message box" (the TR) is not focusable so targeting it with focus() won't have any effect. Focus will be lost. Firefox will keep focus "in place" but that's because Firefox tries to be clever :) with Chrome and IE focus will be lost.
Also, when "click" is triggered, the "update" link will get removed and that HTML portion rebuilt, so there's nothing to focus there. That's why in #31608 we set focus on the checkbox, maybe not nice, but I really don't know what else we could do here.

#9 @jorbin
10 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 32062:

Enable users to initiate a shiny update from plugin detail modal

The plugin detail modal can contain a link to update a plugin. When it does, we should initiate a shiny update.

This relies upon postMessage which isn't available in all browsers, specifically it isn't in IE versions below 8 so this is going to be a progressive enhancement that some small percentage of users will miss out on. These are the same users that can't use the customizer.

Fixes #31739

#10 @jorbin
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

updates-core.php also has a modal that we need to account for. We should disable it on that screen since you can't do a one off shiny update on that screen.

#11 @jorbin
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 32067:

Disable modal initiated shiny updates on wp-admin/update-core.php

We don't have links to update a single plugin on wp-admin/update-core.php so we can't initiate a shiny update there.

Fixes #31739

#12 @pavelevap
10 years ago

I updated to latest trunk and does not work for me. When I click update button from plugin modal, I am redirected to standard 4.1 update page...

#13 @jorbin
10 years ago

@pavelelevap - What browser is this happening on and from which screen are you initiating the request? Is this happening from the src directory or are you using a checkout of core.[svn,git].wordpress.org? If it's a core checkout, does it persist with script_debug enabled?

#14 @pavelevap
10 years ago

Browser: Chrome.
Screen: wp-admin/plugins.php - Show details link for any outdated plugin - click to update in modal window.
I am using src from develop.svn.wordpress.org/trunk.
I added define('SCRIPT_DEBUG', true); but no change.
I tried anonymous window and clearing cache.

Other hints: I am using xampp on Windows, localized version.

#15 @jorbin
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#16 @jorbin
10 years ago

In 32080:

Fix logic for details based shiny updates.

This fixes two problems. TB seems to sometimes strip window.location.search variables after tb ones, so we need to move it forward. Also fixes logic error.

See #31739

#17 @jorbin
10 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 32082:

Disable shiny updates from modal based on parent window

The QS method added in [32067] and modified in [32080] doesn't work when the user changes the tab inside the modal. Instead, let's use the parent window's location.

Fixes #31739

Note: See TracTickets for help on using tickets.