Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.1] Parameters for Module Chromes #23577

Merged
merged 6 commits into from Nov 9, 2021
Merged

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Jan 17, 2019

This isn't ready to be used yet as it needs some discussion first.

Summary of Changes

All it does is looking in the chrome jlayout folders for XML files and loading them into the module edit form.

Testing Instructions

Create an XML in a chrome folder and put some fields into it. As an example create the file templates/cassiopeia/html/layouts/chromes/params.xml and put the following content into it:

<?xml version="1.0" encoding="utf-8"?>
<form>
	<fields name="params">
		<fieldset
				name="advanced">

			<field
					name="cassiopeia_test"
					type="text"
					label="Test Field Cassiopeia"
					showon="style:Cassiopeia-card"
			/>

		</fieldset>
	</fields>
</form>

Expected result

The defined parameter should be available in the module edit form.
Since the example has a showon defined, it will only show up if you have selected the module style "card" from Cassiopeia.

image

Documentation Changes Required

Obviously.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 17, 2019

@ciar4n mentioned in #23570 (comment) the fact that the way it is done here means template devs need to play nice. The showon would be essential and I also think the prefixed field name.

There are some other thoughts which are needed here. Some that come to mind:

  • What happens if the module style is left on inherited. Currently the custom chrome parameters aren't shown the way the showon is written. Would that be acceptable?
  • What should we do with the existing default chrome fields as defined in https://github.com/joomla/joomla-cms/blob/4.0-dev/administrator/components/com_modules/forms/advanced.xml
  • Current code enforces no naming convention for the XML. You can have one file per template were you define all params in one place or one file per chrome or even more. Allowing multiple files would allow the user to add more parameters for his own custom chromes.
  • What should happen if we have multiple fields with the same name but different attributes.

@ciar4n
Copy link
Contributor

ciar4n commented Jan 21, 2019

What happens if the module style is left on inherited. Currently the custom chrome parameters aren't shown the way the showon is written. Would that be acceptable?

Tricky. I guess this means that to show the parameters, a user has to select the chrome from the list, even thou the selected chrome is the same as the inherited. Seems incorrect but not sure how to work around it.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 21, 2019

Seems incorrect but not sure how to work around it.

Yep, I have the same feeling. Imho we can either show all possible parameters, only the default ones or none. We have no way of knowing which is the inherited style, and it may even change when the active template changes.

@ciar4n
Copy link
Contributor

ciar4n commented Jan 22, 2019

Personally, I would lean towards displaying none. Assuming that can be enforced and the template dev can't override it by simply not setting the 'showon'.

@ghost ghost added the J4 Issue label Apr 5, 2019
@ghost ghost removed the J4 Issue label Apr 13, 2019
@ghost ghost changed the title RFC [4.0] Parameters for Module Chromes [4.0] [RFC] Parameters for Module Chromes Apr 19, 2019
@joomla-cms-bot joomla-cms-bot added the RFC Request for Comment label Apr 19, 2019
@roland-d
Copy link
Contributor

roland-d commented Aug 1, 2020

@Bakual Any interest in picking this up and fixing the conflicts?

@Bakual
Copy link
Contributor Author

Bakual commented Aug 2, 2020

Yep, definitively interested, but certainly not in the comins week or two. Maybe later I can fix it,
Since it's an RFC, I would be interested in more opinions on the matter anyway.

@Bakual
Copy link
Contributor Author

Bakual commented Oct 19, 2020

Fixed the conflicts (they were coming from the now merged other PR). Adjusted the PR description.

@punambaravkar
Copy link

I have tested this item ✅ successfully on 67d4641

Thank you for adding Parameters for Module Chromes


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

@priyankaSahutekdi
Copy link

I have tested this item ✅ successfully on 67d4641


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

@HLeithner
Copy link
Member

@Bakual can you merge the current 4.1-dev into it and change the target branch because this is a new feature and it's too late for j4.0.

@Bakual Bakual requested a review from rdeutz as a code owner November 7, 2020 18:52
@Bakual Bakual changed the base branch from 4.0-dev to 4.1-dev November 7, 2020 18:53
@joomla-cms-bot joomla-cms-bot added PR-4.1-dev and removed Language Change This is for Translators labels Nov 7, 2020
@Bakual
Copy link
Contributor Author

Bakual commented Nov 7, 2020

PR is rebased on 4.1-dev now. Code has not changed.

@Bakual Bakual changed the title [4.0] [RFC] Parameters for Module Chromes [4.1] Parameters for Module Chromes Nov 7, 2020
@joomla-cms-bot joomla-cms-bot removed the RFC Request for Comment label Nov 7, 2020
@Quy Quy removed the PR-4.0-dev label Nov 7, 2020
@bembelimen bembelimen added this to the Joomla 4.1 milestone Oct 19, 2021
Skip template if chrome folder doesn't exist
@Bakual
Copy link
Contributor Author

Bakual commented Oct 19, 2021

Removed the use of JForm in favor of Form 😄
Now skipping a template if there is no chrome folder in it (was giving an warning message before)

@chmst
Copy link
Contributor

chmst commented Oct 24, 2021

I like this feature, and it works as described.
As it is a new feature, and documentation is needed, we could do a reordering of the fields, so that it is better understandable?

At least move the field module style at the end, so it is together with newly pluggend in params? @Bakual

@Bakual
Copy link
Contributor Author

Bakual commented Oct 24, 2021

That's a good idea. I moved the style parameter to the end so it's next to the chrome parameters.

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@chmst
Copy link
Contributor

chmst commented Oct 24, 2021

I have tested this item ✅ successfully on 7171044


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

@RickR2H
Copy link
Member

RickR2H commented Oct 24, 2021

I have tested this item ✅ successfully on 7171044

Field shows up and works and parameter is stored in the DB.


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.1 milestone Oct 24, 2021
@RickR2H
Copy link
Member

RickR2H commented Oct 24, 2021

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 24, 2021
@richard67 richard67 added this to the Joomla 4.1 milestone Oct 24, 2021
@bembelimen bembelimen merged commit 16cf06a into joomla:4.1-dev Nov 9, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 9, 2021
@bembelimen
Copy link
Contributor

Thx

@Bakual Bakual deleted the ChromeParams branch November 9, 2021 19:26
@RickR2H
Copy link
Member

RickR2H commented Feb 11, 2022

@Bakual I just noticed that language strings is the params.xml are not working. In your example you use hard coded language strings. It would be nice if language strings can be added so the parameters become Multilingual. Maybe add a new PR. Ping me if you need someone to test 😉


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

@Bakual
Copy link
Contributor Author

Bakual commented Feb 11, 2022

Language strings work. I actually just used the feature this week. Did you put the language strings into the templates template_name.sys.ini language file? Because I think only that one is loaded.

@RickR2H
Copy link
Member

RickR2H commented Feb 12, 2022

@Bakual Looks like I added the translations to the other file without the sys. Thanks for the tip!

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.

None yet