Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#30168] - [4.0] change form in table by form in control-groups

NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar hans2103
hans2103
22 Jul 2020

Pull Request for Issue #29901 .

Summary of Changes

This PR replaces the form-in-table by a form with decent control-groups

Testing Instructions

  • After applying patch running npm run build:js is required
  • Joomla Administrator > System > Update > Joomla > tab = Upload & Update
  • View source and inspect the HTML around the input fields

Actual result BEFORE applying this Pull Request

All form elements placed in a table.

Schermafdruk 2020-07-22 23 13 45

Expected result AFTER applying this Pull Request

All form elements are nicely set in a control-group with decent labels

Schermafdruk 2020-07-22 23 15 20

Documentation Changes Required

nope

avatar hans2103 hans2103 - open - 22 Jul 2020
avatar hans2103 hans2103 - change - 22 Jul 2020
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 22 Jul 2020
Category ⇒ Administration com_joomlaupdate
avatar hans2103 hans2103 - change - 23 Jul 2020
Title
…
:recycle: change form in table by form in control-groups
[4.0] change form in table by form in control-groups
avatar hans2103 hans2103 - edited - 23 Jul 2020
avatar Quy Quy - test_item - 23 Jul 2020 - Tested successfully
avatar brianteeman brianteeman - test_item - 24 Jul 2020 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 24 Jul 2020

I have tested this item ? unsuccessfully on 412d50a

Same problem as in #30171 (comment)

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar hans2103
hans2103 - comment - 24 Jul 2020

I see it happening too... now I have to find out what script is causing this. Help is welcome on this one.

avatar hans2103
hans2103 - comment - 24 Jul 2020

found something:

joomla-cms/build/media_source/com_joomlaupdate/js/default.es5.js

Lines 11 to 19 in 0a611c4

Joomla.extractionMethodHandler = function(element, prefix) {
var displayStyle = element.value === 'direct' ? 'hidden' : 'table-row';
document.getElementById(prefix + '_hostname').classList.add(displayStyle);
document.getElementById(prefix + '_port').classList.add(displayStyle);
document.getElementById(prefix + '_username').classList.add(displayStyle);
document.getElementById(prefix + '_password').classList.add(displayStyle);
document.getElementById(prefix + '_directory').classList.add(displayStyle);
}

avatar hans2103 hans2103 - change - 24 Jul 2020
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 24 Jul 2020
Category Administration com_joomlaupdate ⇒ Administration com_joomlaupdate JavaScript Repository NPM Change
avatar hans2103 hans2103 - change - 24 Jul 2020
The description was changed
avatar hans2103 hans2103 - edited - 24 Jul 2020
avatar hans2103
hans2103 - comment - 24 Jul 2020

Addition for test instructions:

  • After applying patch running npm run build:js is required
avatar Quy Quy - test_item - 24 Jul 2020 - Tested successfully
avatar Quy
Quy - comment - 24 Jul 2020

I have tested this item ✅ successfully on 86fd42d

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar hans2103 hans2103 - change - 24 Jul 2020
Labels Added: NPM Resource Changed
avatar hans2103
hans2103 - comment - 25 Jul 2020

Due to the last request by @Quy in #29901 (comment) and the same JS issues we stumble upon in both this issue as in #30171 I have merged changes from #30171 into this one.

This PR contains four changes:

  • refactor table layout to control-groups for com_joomlaupdate/update
  • refactor table layout to control-groups for com_joomlaupdate/reinstall
  • refactor table layout to control-groups for com_joomlaupdate/upload
  • JS fix to prevent adding class table-row
avatar Quy Quy - test_item - 27 Jul 2020 - Tested successfully
avatar Quy
Quy - comment - 27 Jul 2020

I have tested this item ✅ successfully on 1c7595b

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar david0296 david0296 - test_item - 3 Aug 2020 - Tested successfully
avatar david0296
david0296 - comment - 3 Aug 2020

I have tested this item ✅ successfully on 0a55287

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar johndeutesfeld johndeutesfeld - test_item - 3 Aug 2020 - Tested successfully
avatar johndeutesfeld
johndeutesfeld - comment - 3 Aug 2020

I have tested this item ✅ successfully on 0a55287

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar jiweigert
jiweigert - comment - 3 Aug 2020

Hi,

just my 2 cent: Shouldn't be the ampersand in [code]Upload & Update[/code]?
Or do i still stuck in old HTML times?

Kind regards,

Jörn-Ingo Weigert

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar jiweigert
jiweigert - comment - 3 Aug 2020

Upps, comment scrambled: My comment was targeting on the Legend Text
<legend>Upload & Update<{legend>
to be replaced with &amp; (&)

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar roland-d
roland-d - comment - 4 Aug 2020

@hans2103 When I load the page with your changes I see:
image

After selecting Write files using FTP or Hybrid I see:
image

A few things I notice:

  • Is the table-row class expected?
  • The input fields do not appear

After selecting Write files directly again, I see:
image

There is the class hidden and we have the inline style display: none, is that expected?

I enabled FTP mode in the global configuration and I see this when I go to the Update page:
image

Selecting Write files using FTP I see:
image

Going back to Hybrid the input fields stay small. Going to Write files directly the input fields do not disappear.

avatar roland-d
roland-d - comment - 4 Aug 2020

@jiweigert That change would be one for the language files if it is needed, as far as I see, this is not related to this particular change.

avatar roland-d roland-d - test_item - 4 Aug 2020 - Tested unsuccessfully
avatar roland-d
roland-d - comment - 4 Aug 2020

I have tested this item ? unsuccessfully on 0a55287

Test is not successful as there are some layout issues.

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30168.

avatar roland-d roland-d - alter_testresult - 4 Aug 2020 - roland-d: Tested successfully
avatar roland-d
roland-d - comment - 4 Aug 2020

@hans2103 All is good now, I compiled the CSS instead of the JS. As for not showing the input fields when you choose FTP or Hybrid mode, this is also present without this PR, so I consider it out-of-scope.

avatar roland-d roland-d - change - 4 Aug 2020
Status Pending ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2020-08-04 08:36:49
Closed_By ⇒ roland-d
avatar roland-d roland-d - close - 4 Aug 2020
avatar roland-d roland-d - merge - 4 Aug 2020
avatar Quy
Quy - comment - 4 Aug 2020

@roland-d FTP must be enabled under Global Configuration > Server > Enable FTP for these fields to display. FTP options should be disabled when FTP setting is disabled.

avatar roland-d
roland-d - comment - 5 Aug 2020

@Quy I am aware that FTP must be enabled in the Global Config but showing the options in the upload is confusing. If you show them I would expect the fields there to enter the details. I am still convinced the whole FTP mode should be removed. ?

Add a Comment

Login with GitHub to post a comment