Skip to content

Conversation

@betweenbrain
Copy link
Contributor

Background

With the implementation of using JLayout to render parameters (#2162) support for rendering custom named fieldsets ceased to be supported. This is a documented feature since at least Joomla 2.5 http://docs.joomla.org/Adding_custom_fields_to_the_article_component and is also documented to be suported in 3.1 at http://docs.joomla.org/Adding_custom_fields_to_core_components_using_a_plugin. In fact, the current core user profile plugin would break if JLayout 'joomla.edit.params' was implemented for com_users.user due to the plugin using custom named fieldsets (https://github.com/joomla/joomla-cms/blob/staging/plugins/user/profile/profiles/profile.xml#L3-L4) and the current implementation of JLayout 'joomla.edit.params' being effectively hardcoded to only render fieldsets named params and attribs (https://github.com/joomla/joomla-cms/blob/staging/layouts/joomla/edit/params.php#L15-L18).

Changes

This PR changes the behavior of JLayout 'joomla.edit.params' to render all fieldsets of the form, except those designated with the ignore_fieldsets property.

Affected by this PR are the following backend edit views:

  • com_categories.category
  • com_contact.contact
  • com_content.article
  • com_finder.filter
  • com_menus.item
  • com_modules.module
  • com_newsfeeds.newsfeed
  • com_plugins.plugin
  • com_tags.tag
  • com_templates.style
  • com_weblinks.weblink

Backwards Compatibility

This PR should be 100% backwards compatible while restoring this functionality.

Testing

Apply the patch associated with this PR and review the above views to ensure that all of the currently rendered fieldsets (as tabs in Isis) remain to be rendered. Then, install and plugin that uses a custom named fieldset and ensure that it is rendered at all of the above views as described in Joomla's documentation.

A test plugin can be downloaded from https://github.com/betweenbrain/Joomla-Custom-Field-Plugin-Example/archive/test.zip

@infograf768
Copy link
Member

You may want to look at the various modal.php used in multianguage, when modifying an edit.php
for example:
/administrator/components/com_content/views/article/tmpl/modal.php

@infograf768
Copy link
Member

I meant that any chnage in an edit.php should also be done in the related modal.php

@betweenbrain
Copy link
Contributor Author

Thanks @infograf768! I must have missed one. I'll check that in a bit.

…g custom fieldsets when using JLayout 'joomla.edit.params'
…g custom fieldsets when using JLayout 'joomla.edit.params'
@betweenbrain
Copy link
Contributor Author

Thanks @infograf768 . I think I got them all now.

@b2z
Copy link
Member

b2z commented Sep 24, 2014

@test ok
The custom fieldsets are displayed in the above mentioned views.

This comment was created with the J!Tracker Application at http://issues.joomla.org/.

@roland-d
Copy link
Contributor

roland-d commented Oct 3, 2014

@test: The custom fields show in the views. The only thing I don't know how to test is the modals. Any guidance on how to check those?

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

@infograf768
Copy link
Member

These are used in multilanguage with associations.
Editing an item will display the tab associations where an item can be selected in another language.
Once this is done and saved, that associated item can be edited via the Edit button: it then opens a new window: this is the "modal".

@betweenbrain
Copy link
Contributor Author

Would you all consider this RTC?

@phproberto phproberto added the RTC This Pull Request is Ready To Commit label Oct 16, 2014
@brianteeman
Copy link
Contributor

Newsfeed tabs are no longer being duplicated etc

@betweenbrain
Copy link
Contributor Author

Added the PBF14 label as this should be easy to test and get to RTC again.

@lunalars
Copy link
Contributor

@test not successful

com_categories.category: ok
com_contact.contact: tabs "form" and "display" missing, but tab "metadata" added
com_content.article: ok
com_finder.filter: tab "filter timeline" missing
com_menus.item: tab "associations" duplicated
com_modules.module: ok
com_newsfeeds.newsfeed: ok
com_plugins.plugin: ok
com_tags.tag: tab "metadata" added
com_templates.style: ok

on the above tab "metadata" always contains fields "author" and "robots"

com_weblinks.weblink: tab "metadata" added (fields: "robots" and "contant rights")


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

@lunalars
Copy link
Contributor

on com_contact.contact tab "associations" is duplicated, too

@betweenbrain
Copy link
Contributor Author

Thanks @lunalars! I'm a bit embarrassed for missing those. I'll get this updated in a bit.

@betweenbrain
Copy link
Contributor Author

Thanks again @lunalars, this should be all set now.

@anibalsanchez
Copy link
Contributor

@test OK,

Applied diff, checked views, installed and enabled plugin, and re-checked views.

  • com_categories.category
  • com_contact.contact
  • com_content.article
  • com_finder.filter
  • com_menus.item
  • com_modules.module
  • com_newsfeeds.newsfeed
  • com_plugins.plugin
  • com_tags.tag
  • com_templates.style
  • com_weblinks.weblink


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

@lunalars
Copy link
Contributor

@test not ok - sorry
using com_patchtester and the test plugin mentioned in the test instructions

com_categories.category: tab "associations" duplicated
com_content.article: tab "associations" duplicated
com_menus.item: tab "required settings" added

things have turned around :-)
a noob question: is it ok to test even if travis doesn't like this?


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

@brianteeman
Copy link
Contributor

(re: travis yes its ok to test. sometimes the error with travis is
unrelated to the issue)

On 18 October 2014 08:36, lunalars [email protected] wrote:

@test https://github.com/test not ok - sorry
using com_patchtester and the test plugin mentioned in the test
instructions

com_categories.category: tab "associations" duplicated
com_content.article: tab "associations" duplicated
com_menus.item: tab "required settings" added

things have turned around :-)
a noob question: is it ok to test even if travis doesn't like this?

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/joomla-cms/4284
http://issues.joomla.org/tracker/joomla-cms/4284.


Reply to this email directly or view it on GitHub
#4284 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@infograf768
Copy link
Member

@test
Associations tabs are duplicated
$this->ignore_fieldsets = array('item_associations');
is missing for article edit, category edit, newsfeed edit, same for the modal.php files concerned
Also missing for contact modal.php

Also I suggest to add the code with all fieldsets to be ignored at the top of the files instead of in the middle.

Another matter : is this really B/C?
At seing the changes we have to add to each page concerned, I worry about 3pd components.

rdeutz pushed a commit to rdeutz/joomla-cms that referenced this pull request Oct 24, 2014
…params'. Fixes joomla#4284

Retrieve all fieldsets instead of params and attribs only

Define ignore_fieldsets consistently

Add ignore_fieldsets

Code style

Further implementation of ignore_fieldsets

Restore options tabs in com_content

Update modal.php with related changes to restore support for rendering custom fieldsets when using JLayout 'joomla.edit.params'

Update modal.php with related changes to restore support for rendering custom fieldsets when using JLayout 'joomla.edit.params'
rdeutz pushed a commit to rdeutz/joomla-cms that referenced this pull request Oct 24, 2014
@roland-d
Copy link
Contributor

Removing test results because there are a number of unsuccessful tests and B/C is in question.

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

@betweenbrain
Copy link
Contributor Author

To address the BC concerns, the intent of this PR is to correct a previous backwards incompatible change that broke documented functionality in both Joomla 2.5 and 3.1+. Further details are provided in the description of this PR.

If there is consensus that this would be accepted, I will update this PR to correct any test failures.

@brianteeman
Copy link
Contributor

From your description it sounds like we need this
On 28 Nov 2014 00:13, "Matt Thomas" [email protected] wrote:

To address the BC concerns, the intent of this PR is to correct a previous
backwards incompatible change that broke documented functionality in both
Joomla 2.5 and 3.1+. Further details are provided in the description of
this PR.

If there is consensus that this would be accepted, I will update this PR
to correct any test failures.


Reply to this email directly or view it on GitHub
#4284 (comment).

@roland-d
Copy link
Contributor

roland-d commented Dec 1, 2014

I agree with @brianteeman that it sounds like we need this since B/C broke. @Bakual @chrisdavenport Can this be accepted?

@wilsonge
Copy link
Contributor

@betweenbrain Looks like this is out of sync. I'd also like to see the association bug stuff to be fixed before merging.

The JLayouts were introduced in 3.2 - so if there was a b/c effect of these fields being removed I think we would have seen it with plugin reports. However I agree with Matt that without these a lot of the JForm functionality is lost so it's important to me that we reimplement the ability to do that.

@zero-24
Copy link
Contributor

zero-24 commented Jun 5, 2015

Closing as we have now a new version by @pollen8 here: #7126

@zero-24 zero-24 closed this Jun 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.