Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.authorize.php asks the user for connection details. However, what's extremely confusing (most Drupal developers don't even understand this yet) is that the connection is between the httpd web server process running PHP and the machine hosting the files that httpd is reading. In other words, in the VAST majority of cases, this is going to be "localhost". Prompting the user for this information is only bound to lead to confusion, especially when they see "localhost" as the default and think "wait a second, no, I don't want to update localhost, I want to update my server!"... It'd probably still work if they gave the external hostname for their server, but it's wasted effort, and could potentially cause problems in fact. I consider this a UI bug.
I propose we move both the "host" and "port" settings to the bottom of the form, and put them in a collapsed fieldset called "Advanced settings", much like we have on the install.php UI. Any objections?
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 613912-5.installer-db-settings-collapsed.png | 36.05 KB | dww |
| #5 | 613912-5.installer-db-settings-expanded.png | 64.1 KB | dww |
| #1 | 613912-1.connection_adv_settings.patch | 2.48 KB | dww |
| #1 | 613912-1.connection_adv_settings.before.png | 28.77 KB | dww |
| #1 | 613912-1.connection_adv_settings.after-collapsed.png | 27.45 KB | dww |
Comments
Comment #1
dwwPatch and screenshots (before, after when you first land on authorize.php and the fieldset is collapsed, and after if you expand the fieldset).
There's something stupid going on with garland/minelli here when you expand the fieldset given fixed-width. We could solve that here by forcing these text fields to be a smaller width, or we could fix this for real via #547068: D7UX: use Seven for installation / updates. ;)
Comment #2
Dries CreditAttribution: Dries commentedPersonally, I'm not a fan of nested fieldsets. The outer-most fieldset seems redundant.
Comment #3
dww@Dries: That's something to revisit at #609772: Impossible to extend the FileTransfer class system in contrib since this whole form needs a lot of love, and the outer fieldset is about selecting the connection type. But, I think the advanced settings to hide the host and port is going to be essential for UI, regardless of the outcome of #609772 and I'd rather keep the patches smaller and more self-contained...
Comment #4
Bojhan CreditAttribution: Bojhan commentedIs there any reason to have the Connection settings in a fieldset? As Dries mentioned - it will go against Drupal's patterns to have nested fieldsets, which we have for very good reasons. Beside that, why is this in Garland?
Could we perhaps have some screenshots, of how other interfaces do this - since the confusion is really led by those experiences.
Comment #5
dwwRe: why Garland: as I wrote above, see #547068: D7UX: use Seven for installation / updates
Re: nested fieldsets: I agree, we don't need the outer fieldset, but this is the wrong issue to fix that. We need to do so as part of #609772: Impossible to extend the FileTransfer class system in contrib Meanwhile, use a bit of imagination about how this would look if it's wasn't nested, okay? ;)
Re: Screenshots of other interfaces, you mean like the DB connection settings during the installer?
Comment #6
Bojhan CreditAttribution: Bojhan commentedSo from my understanding.
Yes, nested fieldsets is stupid - not this issue to fix (#609772)
Question, move these settings in a seperate fieldset. Yes.
Be really aware, that we do need to fix the nested fieldset issue though - we should comply with our own standards.
Comment #7
dww@Bojhan: Thanks for the review.
Re: "Be really aware, that we do need to fix the nested fieldset issue though - we should comply with our own standards."
Fear not, #609772: Impossible to extend the FileTransfer class system in contrib is a critical bug (and therefore release blocker). We'll fix it, don't you worry... ;)
Comment #8
webchickI also share the concern that nested fieldsets are a sure-fire sign that #youredoingitwrong UI-wise, and figured breaking these up would be a trivial thing to do, but it turns out it's not because of the way the form builder function is.
I got dww's assurances though that this /will/ be cleaned up in the critical follow-up issue, so for now I've commited this to HEAD since getting that non-required data hidden away is a good UX improvement on its own.