[UX] Update dependency message not very helpful.

[jenlampton]

Describe your issue or idea

When update dependancies are not able to be met, the message presented to the user may be Some of the pending updates cannot be applied because their dependencies were not met. which is not very useful. We do have a collection of useful messages queued and stored, we should present those to the user as well as this generic one.

Steps to test

• Install paging module
• Set the schema version for paging (manually -- to 0)
• attempt to run update.php

Before the PR:

After the PR:

[klonos]

Some feedback based on the before/after screenshots @jenlampton:

1. The "Some of the pending updates ..." message does not make much sense as part of a list. Unless I'm missing something.

2. If we show the specific messages, then do we still need that "No pending updates." there? ...and does it even make sense?

[opi]

remember that list items will be styled as messages (https://github.com/backdrop/backdrop-issues/issues/2857) ;

[opi]

@klonos about your feedbacks :

1. I think the "Some of the pending updates..." message is still needed, as a summary.
2. The "no pending updates" is ok too, as it's unreleated and we could have some pending updates too, see upcoming screenshot

[opi]

Updated PR, rebased on 1.x and with my review concern addressed : https://github.com/backdrop/backdrop/pull/2107

Updated screenshots

[jenlampton]

We need to be careful not to use the word "upgrade" when we mean "update" the distinction between the two is already confusing. Other than that word, I like this change! One thing that worries me a little bit is that it's getting quite verbose. We've learned from usability studies that the more words are in a paragraph, the less likely people are to read it.

A thought: Do we really need the "Some of the..." bullet item if we also have the explanation about paging?

I can also see @klono's point about No pending updates. I would prefer that read 'No other pending updates' in the case where we have a pending update that can't be run.

[olafgrabienski]

We need to be careful not to use the word "upgrade" when we mean "update"

I fully agree, especially on this page because there isn't only the general distinction between updates and upgrades and but also the one between code updates and database updates. To avoid confusion I'd add the suggestion to always mention the word "database" where applicable, e.g.

• Some of the pending database updates cannot be applied ...
• The database thing of paging module cannot be updated ...
• No (other) pending database updates.

[jenlampton]

Some of the pending database updates cannot be applied ...

These updates can also include config files, so are not limited to the database. I think specifying database updates everywhere might be going too far.

[jenlampton]

Okay, I've pushed another update to the PR that now indicates that updates were skipped, but prints the messaging in the fieldset for each update. New screenshots added to the parent issue

I disliked the "No pending updates" that appeared in green before, as though everything is okay. Using the fieldset to say there were 0 pending updates (but with the warning in yellow) I think does a better job of conveying that everything is not okay.

[quicksketch]

Is the screenshot in the summary accurate? It seems odd that there are two message locations, inside and outside of the Available Updates. @opi's screenshots where the update messages are shown in the message area make more sense to me and are more consistent with other UIs. However, I don't think we need the "Some of the pending updates cannot be applied..." message at all. If we're listing each individual module problem, there's no need to include the general message at the top.

[jenlampton]

Okay, I reworked the PR to do actual backdrop_set_messages for each module that has an unmet update requirement, but left the info for how to resolve in the body of the page.

Also: Screenshot replaced. Adding milestone since this one's almost ready.

[klonos]

This could be OT, sorry... OK, I have started testing this and noticed something weird that I thought I'd mention: I have installed devel, paging, and webform modules and have manually set their schema version to 0 in the database. Paging and webform both complain that module can not be updated. Its schema version is 0., but devel seems to somehow not be affected by this change:

[klonos]

...so screenshot above is before. Here's after applying the changes in the PR:

The "4 pending updates" bit clearly applies to the devel module updates, so 👍, but the "2 to be applied, 2 to be skipped" is confusing 🤔

[klonos]

@jenlampton I have filed a PR against your branch here: https://github.com/jenlampton/backdrop/pull/4

• Converts the repeating "The xyz module cannot be updated" warnings to a single "The following modules cannot be updated:" warning, followed by a list of such modules.
• Adds human-readable names to modules.
• Moves the module name titles outside the warnings (as proper list items), which make the list within the fieldset look more consistent
• Moves the warnings "inline", within the respective module name list item.
• Wraps some strings within t().

Screenshot:

Now silly as it may be, I could not for the love of Backdrop make those bullet points next to the warning lis go away. I have tried adding list-style-type: none; and/or list-style-image: none; to the lis and/or the uls. I have tried adding the code in /core/modules/system/css/messages.theme.css and then also in /core/themes/seven/css/style.css, but nothing seems to work ...no matter how CSS-specific I try to be 😓. Hewp pwease?

[klonos]

...also needs some spacing between the fieldset label and the fieldset content.

[quicksketch]

Here's a screenshot of the current PR:

I think we have a few issues here. The difference in fonts between applied and non-applied updates bothers me, as does the very long fieldset title that wraps and reveals some line-spacing issues. It would be preferable IMO to display working and non-working updates with the same markup so that they display in the same way.

The styling changes proposed by @klonos in the two comments above I think are excessive. Just including an explanation seems fine to me.

[laryn]

It's been a few years but I was trying to track down what the problem was on a local site in development and came across this issue. I'd like to help unstick it. I manually patched locally and here's how things look with the PR linked at the top of the issue:

Before:

After:

I think having the warning icons and colors inside the dropdown is too much but after removing that it feels like it would be pretty close to ready.

[klonos]

I think having the warning icons and colors inside the dropdown is too much...

When you have only problematic updates listed in that report, then the above might be true. With a mix of pending and skipped updates though, and especially if you have a list with many modules and updates in the fieldset, I imagine it would be hard to immediately distinguish the problematic ones in the list.

Here's what it would look like with the problematic updates being in plain text instead of a warning, when only a single module is problematic:

...and the same if there are multiple problematic module updates:

Screenshots above are from a fresh PR, based on @jenlampton's + rebased + the tweaks I previously mentioned in my comment above, back in Jan 2019.

If everyone agrees that the warnings are indeed too much, then I would like us to try to style this summary similarly to the status report page, where the icons we are using for errors/warnings are more subtle I believe. That would be a UI pattern similar to the various errors/warnings we are throwing in the installer as well (basically the same visual result as the 3rd design proposal in #2873). I'd like to give that a go over this weekend.

[laryn]

I’m in favor of more subtle 👍

[klonos]

OK, I haven't pushed anything in the PR branch yet, but here's where this is at currently on my local:

So I am reusing theme_status_report() for the table here, which by default bubbles errors/warnings at the top of the table, which is what we want here as well I believe (previously the pending updates were being listed alphabetically, by module name).

There are also some edge cases where we are skipping updates, with the following text being appended to each individual update:

• This update will be skipped due to the following missing dependencies: ...
• This update will be skipped due to an error in the module code.

I have not adjusted the code yet to fully support those cases and look nice and all, but here's where I'm currently at with that (manually causing these errors for randomly picked updates):

• The modules that contain problematic updates (along with non-problematic ones) also bubble up as warnings 👍🏼
• Still unsure how to denote the problematic updates within the bulleted list. Currently simply highlighting them in red color, but thinking perhaps something along the lines of red ✗s or !s, similar to this:
• The x pending updates info needs to be updated to denote the number of problematic updates. Something like 5 pending updates and 2 to be skipped.
• The overall warning message at the top of the table also needs to be updated, perhaps with something like x modules have some updates that will be skipped.
• Previously, all pending updates were listed on this page. Now that the non-problematic updates are listed as "info" rows, the details are collapsed within the more/less toggle by default. Do people find that a good change, or a bad change? Perhaps I need to figure out a way to auto-expand those despite not being warnings.

So WIP, but getting there.

[klonos]

It bothers me that the "Cancel" link always takes people to the front page regardless of where they originated from, but it's too late to use $_SERVER['HTTP_REFERER'] here - it needs to be done in the previous step (#512) and carried over here (either via a custom header or via the $query parameter in backdrop_current_script_url()). So I'm leaving that as a follow-up.

Edit: I've created #6289 for that ^^

[klonos]

With #2873 now merged, the changes in the PR for this issue here can be reduced.

[laryn]

I would love to see this one get in to the next release. These cryptic messages come up regularly...

[stpaultim]

@klonos Can you rebase/update your PR?

[laryn]

@klonos @stpaultim I've just rebased the PR.

[stpaultim]

The rebase was probably not so important, since it seems this PR must be tested locally, so that one can make direct changes to the database and load old versions of modules.

Having said that, I think that we have good screenshots to show the main issues being discussed and folks can provide feedback based upon those screenshots.

I was able to test the current PR. But, it seems that PR is aready out of date.

The current PR looks like this:

Apparently, @klonos has also done the following locally, but these changes are not in the PR.

I think it would be nice if we could get the newer and more subtle version of this going. @klonos are you able to push this code so that others could take over and work on this PR even if you are not available?