Hello @hgh-esn

It's a bit hard to follow your changes when I look into the tabulator "Files changed" on GitHub because you exchange the whole files via upload.

Normally one creates a custom branch based upon the current 4.0.0-dev branch. Then changes the code inside the custom branch and commits the changes to the 4.0.0-dev branch here.

See an example here what's the result: https://github.com/joomla/joomla-cms/pull/24968/files
Just changed lines are highlighted.

On the other hand: You don't have to create new prs if you have to make changes. You can commit additional changes to the same pr.

Maybe somebody can help you here how to do that if you tell us which tools you use (I only use the GUI of GitHub. Thus I can't help.)

But again: Just a private note from my side. I don't have to decide that.

@hgh-esn Check your IDE settings. We use tabs to indent code. Your IDE likely uses spaces which results in changes in every line instead of only those you touched.

Beside this, postgresql schema updates are wrong, they are same as the mysql files and so have wrong syntax e.g. for qouting.

Beside this, postgresql schema updates are wrong, they are same as the mysql files and so have wrong syntax e.g. for qouting.

Why don't you just fix things and then when ready upload it at once with 1 commit for all? Why for every little change 1 comming with same message "Add files via upload"?
I hope that if this PR ever gets merged, then it will be merged with 1 squash commit so we don't see the 9845 (or so) commits "Add files via upload" which it will have at the end in Joomla history for the next 5 years.

why this: --- continuous-integration/drone/pr — Build is failing ?

Hello,

and what is going on next here?

@hgh-esn Every Pull Request needs to successfully test, then Release Lead @wilsonge decide if and when this get merged.

@franz-wohlkoenig

... and who will do the test?

Everyone can do a test, as all are Volunteers.

@franz-wohlkoenig

Everyone can do a test, as all are Volunteers.

Please tell me more about the process.
I think the pr is coded and the checks are ok. How does it go from here?
a) Where is the test platform? Is this a separate environment or is the joomla4 alpha?
b) If so, how does the code get in there?
c) Who decides (@wilsonge?), when the feature in joomla4 alpha? is taken over.
d) Do I have to do anything about this? Or is it enough to wait?

After 2 successfully Tests Pull Request get Status "Ready to Commit". Next Step Release Lead (@wilsonge) decide to merge or not.

Please separate the last commit to its own branch.

@Quy

Please separate the last commit to its own branch.

Sorry for that, it was my fault. Git is to strange to me.
How can I seperate it? I'm a bit confused!

@franz-wohlkoenig There is no test item for this in the issue tracker. Any idea how to fix that? PR looks good to me now and I have just tested with success.

@franz-wohlkoenig There is no test item for this in the issue tracker. Any idea how to fix that? PR looks good to me now and I have just tested with success.

There is no test item for this in the issue tracker. Any idea how to fix that?

Happens sometimes, don't know why but calling @mbabker helps always :-)

@richard67
Thanks to you and all the team(s). That was - in all - a strongly birth for this little change.
I felt as falling in every trap that was possible. ;)

But lately it is good to read that you tested it now successfully.

And why do I find often "JText_(xxxx" statements in Joomla and now it should be reduced to "Text_(" ?
Can anyone explain that?

Joomla 3 used JText
Joomla 4 uses Text

There is currently one instance of JText to be updated

@brianteeman
Thanks for the explanation.

I think there were good reasons for that, but it makes the code not easier to read, because the assignment to Joomla is no longer directly recognizable.

@hgh-esn What happened? Clicked wrong button? If you did not close this PR intentionally, you should reopen it, because it is closed now.

@richard67
you'r right, I clicked wrong button? Sorry

@mbabker Can you help with this PR not having a test item in the issue tracker?

@richard67 Can you please tell me what that is ... a test-item in the issue tracker?

@hgh-esn Issues and pull requests not only appear on GitHub but also on the Joomla issue tracker, https://issues.joomla.org/.

For pull requests there is a button "Test this", which then allows someone who tested a PR to set the result accordingly.

It needs successful tests for a PR to get ready to be merged (ready to commit, RTC).

Sometimes this issue tracker has a problem with a PR, like it is with yours. The button for the test is missing in the issue tracker, so someone who tests your PR can not set the result there.

This is not your fault, it can happen sometimes with that issue tracker in recent times.

@richard67 thanks for that.

@richard67

... PS: Can I contact you via your homepage(email) and later by phone.
I think it is better to talk about that here by phone.

@hgh-esn No, better not. If you need some more information about Joomla coding stuff, check documentation e.g. here: https://docs.joomla.org/Git_for_Coders.

@mbabker
Thanks for setting up the "Test this" button.

@richard67
You can use it now.

@hgh-esn Regarding functionality and code style, your PR seems to be ok now. But there is an issue regarding consistency of UI compared to other note fields in the list views, from my point of view.

The note field of site modules is shown in the modules list as follows in a div, with text in brackets and no <strong> attribute:

The note field of categories is shown in the categories list in the same way, but in a span instead of a div so it appears in the same line:

For articles it is same as for categories except that there is a break-word class in addition:

So there is no 100% consistent way, but at least they have in common that nowhere is used <strong>.

With this PR, another new way to show the note in the list view is added for plugins:

I think it does not look bad, but it increases inconsistency in UI, so I suggest to change it so it is the same as with site modules.

But that's only my opinion.

@infograf768 What do you suggest?

We must have a consistent UI

@brianteeman That was the intention of my comment, and that's why I wait with testing. But as you see we are not 100% consistent at other places, too. I suggest to do it in this PR in the same way as in the site modules, and clean up other inconsistencies with a separate PR. Do you agree? If not, what do you suggest?

I rather like the way the Note is displayed here, i.e. below. It is much clearer to me.
We could, in another PR, normalize this placement.

Just found another bug.
Edit a Fields plugin. One gets 2 Note fields.

@infograf768 I think that was the reason for the change in file "layouts/joomla/edit/global.php". Did you test with this change included? I did, and I don't have that issue with duplicate note field in a fields plugin, e.g. "Fields - Integer".

@infograf768 And regarding IU: I agree with you that it looks better below than beside in the list view, but what about the <strong>, shouldn't that be removed? I think it would be ok if done like for the site modules.

I think that was the reason for the change in file "layouts/joomla/edit/global.php". Did you test with this change included? I did, and I don't have that issue with duplicate note field in a fields plugin, e.g. "Fields - Integer".

You are correct.

And regarding IU: I agree with you that it looks better below than beside in the list view, but what about the <strong>, shouldn't that be removed? I think it would be ok if done like for the site modules.

Agree too.

@infograf768 The fact that it is in brackets in the list view does not look nice. I think we should remove those brackets everywhere later in another PR.

Regarding the change in file "layouts/joomla/edit/global.php": I think it should not use negative logic, i.e. not check for $component !== 'com_plugins' but for $component === 'com_plugins' and exchange if and else code accordingly. Do you agree?

Indeed, better positive than negative.
Not sure I understand about the brackets.

@infograf768 In site menus list: (Note: blabla). In categories list: (Alias: uncategorized, Note: blabla). I mean the ( ) brackets. See my screenshot above.

Yep, once we modify the display of the Note to be below the title, the parentheses are unnecessary
? https://practicaltypography.com/parentheses-brackets-and-braces.html

Point is we should be consistent, and that means text JGLOBAL_LIST_NOTE should be used in the list view and not JFIELD_NOTE_LABEL in order to be consistent with the other list views. I will make a review with change request for that. Later we can change these texts for removing the brackets in another PR.

Completed testing instructions

Completed testing instructions

Please revert changes to the global.php layout file. There should be no component-specific code like this. Instead, you can either remove the field from edit.php and have it rendered by global.php. Or set $this->fields property with form field names excluding note.

@SharkyKZ @infograf768

Do not really know what "SharkyKZ" means in details.
I'm confused and out here until you cleared it internally.

@SharkyKZ means undo the change to global.php and modify edit.php to get something like

			<div class="col-md-3">
				<div class="card card-light">
					<div class="card-body">
						<?php
						// Set main fields.
						$this->fields = array(
							'enabled',
							'access',
						);

						echo LayoutHelper::render('joomla.edit.global', $this); ?>

						<div class="form-vertical form-no-margin">
							<div class="control-group">
								<div class="control-label">
									<?php echo $this->form->getLabel('ordering'); ?>
								</div>
								<div class="controls">
									<?php echo $this->form->getInput('ordering'); ?>
								</div>
							</div>
							<div class="control-group">
								<div class="control-label">
									<?php echo $this->form->getLabel('folder'); ?>
								</div>
								<div class="controls">
									<?php echo $this->form->getInput('folder'); ?>
								</div>
							</div>
							<div class="control-group">
								<div class="control-label">
									<?php echo $this->form->getLabel('element'); ?>
								</div>
								<div class="controls">
									<?php echo $this->form->getInput('element'); ?>
								</div>
							</div>
							<div class="control-group">
								<div class="control-label">
									<?php echo $this->form->getLabel('note'); ?>
								</div>
								<div class="controls">
									<?php echo $this->form->getInput('note'); ?>
								</div>
							</div>
						</div>
etc.

@infograf768 @SharkyKZ @richard67

That is exactly that what it was in my early codings.
When reversing the global.php we go back to that, what you have seen this morning in your test.
The note input-field will then be double displayed. (I don't know why ?)
But with the changes in global.php the doubling effect has gone away.

Btw.: The note field is in edit.php as you can see at the end.

@SharkyKZ
Please make me a good functional code-tip to get your idea to the right.

Btw.

Or set $this->fields property with form field names excluding note.

A note is a note.

Is not related to your PR or the changes, is a general problem with AppVeyor it seems.

@hgh-esn
test my code above after reverting the changes in global.php
I tested it here and the Note field is displayed only once, including for the Fields plugin.

						<?php
						// Set main fields.
						$this->fields = array(
							'enabled',
							'access',
						);

						echo LayoutHelper::render('joomla.edit.global', $this); ?>

is taking care of that.

						<?php
						// Set main fields.
						$this->fields = array(
							'enabled',
							'access',
						);

						echo LayoutHelper::render('joomla.edit.global', $this); ?>

@infograf768
Thanks for testing and your experiments with the code.

I'll do that changes at the afternoon. Now I have no time.
But one question there is.
Days ago, we had a discussion about using "php short array syntax".
When I rebuild the global.php should I use the "standard php array syntax" as used before or the new prefered "short array" variant?
The same is in edit.php, for the new statement $this->fields = array('enabled','access',);?or $this->fields = ['enabled','access',];?

I ask this, to avoid any later upcoming discussion about that. ;)

190601,19:30:00
As receiving no answer, I decide it to do it in standard-array-syntax. (Think that is the better readable code).

When you “rebuild” global.php, just use the existing 4.0 global.php. it is a revert, not really a rebuild.

@infograf768 @richard67
Changes are done - testing can go further on.

I have tested this item ✅ successfully on 2b77e96

OK now. I propose some small glitches to be modified later in another PR.

I have tested this item ✅ successfully on 2b77e96

OK now. I propose some small glitches to be modified later in another PR.

@infograf768
Thanks for that good news.
It was a strongly birth to get it to run but, now - I hope - it seams ok.
Who will be the second tester?
Enjoy a good sunny sunday.

Who will be the second tester?

a Volunteer which take Time and is interestet to test.

@franz-wohlkoenig

Does that mean, that if no one feels called, then the PR is hanging around here endlessly.
Can the PR creator also do a second test to push the PR and can he then be the second tester?

Note: In my previous company, there was a test team that carries out such activities. The creator was excluded from it.

Does that mean, that if no one feels called, then the PR is hanging around here endlessly.

Correct. All are Volunteers.

Can the PR creator also do a second test to push the PR and can he then be the second tester?

Creator test can't count as the Project expect that the Pull Request is tested by Creator before creating the PR.

I have tested this item ✅ successfully on 2b77e96

Note field for plugins works.

Display in the list view is consistent with other list views having notes, e.g. site menus.

The display of notes values in list views is not nice in general and should be improved consistently for all list views with another, future PR.

@hgh-esn The reason why you had to run through many code reviews was mainly caused by your implementation and especially at the beginning in the way how you worked, that you committed code which obviously could not work, like e.g. the postgresql stuff.

As Franz mentioned above, we expect that when someone proposes a PR, he has already tested his PR so it will not contain failures.

Beside this, we (volunteers) have to make sure that Joomla code has a certain quality, and so there are code style rules and architectural aspects to be mentioned. It is not enough if a new code just functionally does what it shall do. The code has to have the required quality, UI consistency is also an aspect.

Not everbody of us has the same skills and focus, and so it can happen that one of us (e.g. me) is happy with your PR, but then someone who knows more about Joomla architecture than I do (e.g. SharkyKZ) finds another thing to be improved.

And so yes, it can be hard at the beginning to go through all those reviews, but it is required because we want to have well tested code in a good quality so it is better maintainable.

After a while, people get familiar with that and it will be easier.

Regarding code style you can find some documentation here:

https://developer.joomla.org/coding-standards/basic-guidelines.html

But it also possible to check the log files of drone when after a commit, drone failed. If section phpcs is red, there are code style errors, and a click on that section will show the log file.

In general some links to further documents about contributing to Joomla can also be found on this German start page:

https://docs.joomla.org/Portal:Joomla!_Code_Contributors/de.

Status "Ready To Commit".

@richard67

In your previous explanation, of course, you are right, but allow me some personal views on this:

1. Central Joomla Code: 'global.php'
   It would be very nice to read more about as inline-comments, how it is used properly . Since I did not see the exact function there, I got to the solution with the "code-switch" (if .... === com_plugin)
   then .... else ....;.

That this was different to that, as SharkeyKZ has described, I could have found synonymous, if I had checked in module notes. Unfortunately I have not - my mistake.

But as I said, all that could be avoided, if one would document a bit more in the header of global.php, what is done there by default and how one can possibly influence the behavior from the outside.
Such inline comments can often be very helpful.

2. .... we expect that when someone proposes a PR, he has already tested his PR so it will not contain failures.

I tested everything in my personal environment before handing it over to you. It worked for me, there was - except for this small DB definition problem - no really functional errors. The note could be displayed in both affected views.

Ok, everything that has developed after that is not that great and probably had to come like that. Some discussions (short-array-code, etc.) were - in my opinion - but also unnecessary. This spelling may be for expert programmers, but it does not make the code more readable.
If you load such a code into an editor with syntax highlighting, you will not be happier.

But "sponge over it", that's all now past.

If you want to tackle the topic: notes and their mapping in Joomla again, I offer my further assistance.

I think we should first

• find out where the affected areas are.
• Then you have to specify what the appearance of the note (s) in Joomla should look like.
• only then should the technical implementation take place.

Should I open an issue in J4?

Why closed? It was RTC, means ready to commit. You hit the wrong button accidently?

@hgh-esn Why closed? It was RTC, means ready to commit. You hit the wrong button accidently?

Oh sorry for that !
Ist's a mess in GitHub, that you can close a PR without a second okay.

@franz-wohlkoenig Could you add back RTC label? Author has accidently closed the PR.

Status "Ready To Commit".

@infograf768 @richard67

I try to setup a joomla environment to test the PR myself.
I worked as described in this dok.
I made the download from the joomla staging-environment and have that installed. It runs, but it is a J3.9.7-dev environment.
And here is my question:
Do I have to migrate from J3.9.7-dev to J4-dev first?
If so, that isn't described in that doku, or I haven't found anything about it. ;)

What is your install-process for that patch in details?

@infograf768

I did that also. But I get this when starting the backend.

That is normal. One has to use Composer and NPM. This PR is not the place to explain the HOWTO.

@hgh-esn Find here a link to a document in German language about how to set up your development environment for J4, including the composer and npm stuff which Jean-Marie mentioned:
https://docs.joomla.org/J4.x:Setting_Up_Your_Local_Environment/de

Note also that you should not try to update current staging or 3.9.x to 4.0 with the Joomla Update function. This currently is not intended to work. Later update from 3.10 to 4.0 will me made possible.

@richard67 Thanks for the link, but that is quit different to that, what is written down in this dok. And all together it looks not as easy as a normal tester is willing it to.
It looks very abstract, because of having to install a lot of additional software, with their handling / parameterization a normal tester may be a bit overwhelmed.
At any rate, it was not even fast. I'll just wait for the "Nightly Build Version", where I can see the result.

Allow me one more question:
When will the PR be transferred to the J4 code? "Ready to comit" is it now.

Does anyone have to do anything or is this automatic?

@hgh-esn thiswill be done by Release Lead @wilsonge if he decide to merge the PR. A RTC is no automatic that a PR get in core.

@wilsonge
Can you take a decision on this one?

Thanks! Sorry for the delay merging